Skip to content

Commit

Permalink
Merge pull request #18059 from emberjs/tracked/flush-observers-synchr…
Browse files Browse the repository at this point in the history
…onously

[FEATURE] Adds sync observers back to tracked properties
  • Loading branch information
rwjblue authored May 29, 2019
2 parents 1cb5f67 + 0ce0174 commit 70bcd9f
Show file tree
Hide file tree
Showing 16 changed files with 237 additions and 106 deletions.
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

0 comments on commit 70bcd9f

Please sign in to comment.