Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Adds sync observers back to tracked properties #18059

Merged
merged 1 commit into from
May 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/@ember/-internals/environment/lib/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,20 @@ export const ENV = {
*/
_JQUERY_INTEGRATION: true,

/**
Whether the app defaults to using async observers.

This is not intended to be set directly, as the implementation may change in
the future. Use `@ember/optional-features` instead.

@property _DEFAULT_ASYNC_OBSERVERS
@for EmberENV
@type Boolean
@default false
@private
*/
_DEFAULT_ASYNC_OBSERVERS: false,

/**
Controls the maximum number of scheduled rerenders without "settling". In general,
applications should not need to modify this environment variable, but please
Expand Down
26 changes: 22 additions & 4 deletions packages/@ember/-internals/meta/lib/meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,15 @@ interface StringListener {
target: null;
method: string;
kind: ListenerKind.ADD | ListenerKind.ONCE | ListenerKind.REMOVE;
sync: boolean;
}

interface FunctionListener {
event: string;
target: object | null;
method: Function;
kind: ListenerKind.ADD | ListenerKind.ONCE | ListenerKind.REMOVE;
sync: boolean;
}

type Listener = StringListener | FunctionListener;
Expand Down Expand Up @@ -528,13 +530,14 @@ export class Meta {
eventName: string,
target: object | null,
method: Function | string,
once: boolean
once: boolean,
sync: boolean
) {
if (DEBUG) {
counters!.addToListenersCalls++;
}

this.pushListener(eventName, target, method, once ? ListenerKind.ONCE : ListenerKind.ADD);
this.pushListener(eventName, target, method, once ? ListenerKind.ONCE : ListenerKind.ADD, sync);
}

removeFromListeners(eventName: string, target: object | null, method: Function | string): void {
Expand All @@ -549,7 +552,8 @@ export class Meta {
event: string,
target: object | null,
method: Function | string,
kind: ListenerKind.ADD | ListenerKind.ONCE | ListenerKind.REMOVE
kind: ListenerKind.ADD | ListenerKind.ONCE | ListenerKind.REMOVE,
sync = false
): void {
let listeners = this.writableListeners();

Expand Down Expand Up @@ -585,9 +589,11 @@ export class Meta {
target,
method,
kind,
sync,
} as Listener);
} else {
let listener = listeners[i];

// If the listener is our own function listener and we are trying to
// remove it, we want to splice it out entirely so we don't hold onto a
// reference.
Expand All @@ -598,8 +604,20 @@ export class Meta {
) {
listeners.splice(i, 1);
} else {
assert(
`You attempted to add an observer for the same method on '${
event.split(':')[0]
}' twice to ${target} as both sync and async. Observers must be either sync or async, they cannot be both. This is likely a mistake, you should either remove the code that added the observer a second time, or update it to always be sync or async. The method was ${method}.`,
!(
listener.kind === ListenerKind.ADD &&
kind === ListenerKind.ADD &&
listener.sync !== sync
)
);

// update own listener
listener.kind = kind;
listener.sync = sync;
}
}
}
Expand Down Expand Up @@ -761,7 +779,7 @@ export class Meta {
result = [] as any[];
}

result.push(listener.event);
result.push(listener);
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions packages/@ember/-internals/metal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,7 @@ export { default as getProperties } from './lib/get_properties';
export { default as setProperties } from './lib/set_properties';
export { default as expandProperties } from './lib/expand_properties';

export {
addObserver,
activateObserver,
removeObserver,
flushInvalidActiveObservers,
} from './lib/observer';
export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
export { tagForProperty, tagFor, markObjectAsDirty, UNKNOWN_PROPERTY_TAG } from './lib/tags';
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/-internals/metal/lib/chain-tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function finishLazyChains(obj: any, key: string, value: any) {
}

if (value === null || (typeof value !== 'object' && typeof value !== 'function')) {
lazyTags.clear();
lazyTags.length = 0;
return;
}

Expand Down
15 changes: 14 additions & 1 deletion packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
isClassicDecorator,
} from './descriptor_map';
import expandProperties from './expand_properties';
import { setObserverSuspended } from './observer';
import { defineProperty } from './properties';
import { notifyPropertyChange } from './property_events';
import { set } from './property_set';
Expand Down Expand Up @@ -667,7 +668,19 @@ export class ComputedProperty extends ComputedDescriptor {
let hadCachedValue = cache.has(keyName);
let cachedValue = cache.get(keyName);

let ret = this._setter!.call(obj, keyName, value, cachedValue);
let ret;

if (EMBER_METAL_TRACKED_PROPERTIES) {
setObserverSuspended(obj, keyName, true);

try {
ret = this._setter!.call(obj, keyName, value, cachedValue);
} finally {
setObserverSuspended(obj, keyName, false);
}
} else {
ret = this._setter!.call(obj, keyName, value, cachedValue);
}

// allows setter to return the same value that is cached already
if (hadCachedValue && cachedValue === ret) {
Expand Down
5 changes: 3 additions & 2 deletions packages/@ember/-internals/metal/lib/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export function addListener(
eventName: string,
target: object | Function | null,
method?: Function | string,
once?: boolean
once?: boolean,
sync = true
): void {
assert(
'You must pass at least an object and event name to addListener',
Expand All @@ -53,7 +54,7 @@ export function addListener(
target = null;
}

metaFor(obj).addToListeners(eventName, target, method!, once === true);
metaFor(obj).addToListeners(eventName, target, method!, once === true, sync);
}

/**
Expand Down
78 changes: 53 additions & 25 deletions packages/@ember/-internals/metal/lib/mixin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
@module @ember/object
*/
import { ENV } from '@ember/-internals/environment';
import { Meta, meta as metaFor, peekMeta } from '@ember/-internals/meta';
import {
getListeners,
Expand Down Expand Up @@ -395,20 +396,23 @@ if (ALIAS_METHOD) {
};
}

function updateObserversAndListeners(
obj: object,
key: string,
paths: string[] | undefined,
updateMethod: (
obj: object,
path: string,
target: object | Function | null,
method: string
) => void
) {
if (paths) {
for (let i = 0; i < paths.length; i++) {
updateMethod(obj, paths[i], null, key);
function updateObserversAndListeners(obj: object, key: string, fn: Function, add: boolean) {
let observers = getObservers(fn);
let listeners = getListeners(fn);

if (observers !== undefined) {
let updateObserver = add ? addObserver : removeObserver;

for (let i = 0; i < observers.paths.length; i++) {
updateObserver(obj, observers.paths[i], null, key, observers.sync);
}
}

if (listeners !== undefined) {
let updateListener = add ? addListener : removeListener;

for (let i = 0; i < listeners.length; i++) {
updateListener(obj, listeners[i], null, key);
}
}
}
Expand All @@ -420,13 +424,11 @@ function replaceObserversAndListeners(
next: Function | null
): void {
if (typeof prev === 'function') {
updateObserversAndListeners(obj, key, getObservers(prev), removeObserver);
updateObserversAndListeners(obj, key, getListeners(prev), removeListener);
updateObserversAndListeners(obj, key, prev, false);
}

if (typeof next === 'function') {
updateObserversAndListeners(obj, key, getObservers(next), addObserver);
updateObserversAndListeners(obj, key, getListeners(next), addListener);
updateObserversAndListeners(obj, key, next, true);
}
}

Expand Down Expand Up @@ -845,6 +847,8 @@ if (ALIAS_METHOD) {
// OBSERVER HELPER
//

type ObserverDefinition = { dependentKeys: string[]; fn: Function; sync: boolean };

/**
Specify a method that observes property changes.

Expand All @@ -870,24 +874,48 @@ if (ALIAS_METHOD) {
@public
@static
*/
export function observer(...args: (string | Function)[]) {
let func = args.pop();
let _paths = args;
export function observer(...args: (string | Function)[]): Function;
export function observer(definition: ObserverDefinition): Function;
export function observer(...args: (string | Function | ObserverDefinition)[]) {
let funcOrDef = args.pop();

assert(
'observer must be provided a function or an observer definition',
typeof funcOrDef === 'function' || (typeof funcOrDef === 'object' && funcOrDef !== null)
);

let func, dependentKeys, sync;

if (typeof funcOrDef === 'function') {
func = funcOrDef;
dependentKeys = args;
sync = !ENV._DEFAULT_ASYNC_OBSERVERS;
} else {
func = (funcOrDef as ObserverDefinition).fn;
dependentKeys = (funcOrDef as ObserverDefinition).dependentKeys;
sync = (funcOrDef as ObserverDefinition).sync;
}

assert('observer called without a function', typeof func === 'function');
assert(
'observer called without valid path',
_paths.length > 0 && _paths.every(p => typeof p === 'string' && Boolean(p.length))
Array.isArray(dependentKeys) &&
dependentKeys.length > 0 &&
dependentKeys.every(p => typeof p === 'string' && Boolean(p.length))
);
assert('observer called without sync', typeof sync === 'boolean');

let paths: string[] = [];
let addWatchedProperty = (path: string) => paths.push(path);

for (let i = 0; i < _paths.length; ++i) {
expandProperties(_paths[i] as string, addWatchedProperty);
for (let i = 0; i < dependentKeys.length; ++i) {
expandProperties(dependentKeys[i] as string, addWatchedProperty);
}

setObservers(func as Function, paths);
setObservers(func as Function, {
paths,
sync,
});
return func;
}

Expand Down
Loading