diff --git a/packages/@ember/-internals/meta/lib/meta.ts b/packages/@ember/-internals/meta/lib/meta.ts index ff975cfbcdf..8a9d2bdbc4b 100644 --- a/packages/@ember/-internals/meta/lib/meta.ts +++ b/packages/@ember/-internals/meta/lib/meta.ts @@ -14,6 +14,7 @@ export interface MetaCounters { deleteCalls: number; metaCalls: number; metaInstantiated: number; + reopensAfterFlatten: number; } let counters: MetaCounters | undefined; @@ -25,6 +26,7 @@ if (DEBUG) { deleteCalls: 0, metaCalls: 0, metaInstantiated: 0, + reopensAfterFlatten: 0, }; } @@ -42,7 +44,7 @@ const enum MetaFlags { META_DESTROYED = 1 << 2, } -export const enum ListenerKind { +const enum ListenerKind { ADD = 0, ONCE = 1, REMOVE = 2, @@ -91,7 +93,7 @@ export class Meta { _listeners: Listener[] | undefined; _listenersVersion = 1; _inheritedEnd = 0; - _inheritedVersion = 0; + _flattenedVersion = 0; // DEBUG _values: any | undefined; @@ -540,10 +542,35 @@ export class Meta { } } - // check if we our meta is owned by a prototype - // if so, our listeners are inheritable so check if we have - // cached our flattened listeners, if so then clear the inherited listeners - // and bump the global version count + /** + Flattening is based on a global revision counter. If the revision has + bumped it means that somewhere up the inheritance chain something has + changed, so we need to reflatten everything. This can only happen if: + + 1. A meta has been flattened (listener has been called) + 2. The meta is a prototype meta with children who have inherited its + listeners + 3. A new listener is subsequently added to the meta (e.g. via `.reopen()`) + + This is a very rare occurence, so while the counter is global it shouldn't + be updated very often in practice. + */ + private _shouldFlatten() { + return this._flattenedVersion < currentListenerVersion; + } + + private _isFlattened() { + // A meta is flattened _only_ if the saved version is equal to the current + // version. Otherwise, it will flatten again the next time + // `flattenedListeners` is called, so there is no reason to bump the global + // version again. + return this._flattenedVersion === currentListenerVersion; + } + + private _setFlattened() { + return this._flattenedVersion = currentListenerVersion; + } + private writableListeners(): Listener[] { let listeners = this._listeners; @@ -551,39 +578,54 @@ export class Meta { listeners = this._listeners = [] as Listener[]; } - if (this.source === this.proto && this._inheritedVersion > 0) { + // Check if the meta is owned by a prototype. If so, our listeners are + // inheritable so check the meta has been flattened. If it has, children + // have inherited its listeners, so bump the global version counter to + // invalidate. + if (this.source === this.proto && this._isFlattened()) { + if (DEBUG) { + counters!.reopensAfterFlatten++; + } + currentListenerVersion++; } return listeners; } - protected flattenedListeners(): Listener[] | undefined { - let parent = this.parent; + private flattenedListeners(): Listener[] | undefined { + if (this._shouldFlatten()) { + let parent = this.parent; - if (parent !== null && this._inheritedVersion < currentListenerVersion) { - // compute - let parentListeners = parent.flattenedListeners(); + if (parent !== null) { + // compute + let parentListeners = parent.flattenedListeners(); - if (parentListeners !== undefined) { - let listeners = this._listeners; + if (parentListeners !== undefined) { + let listeners = this._listeners; - if (listeners === undefined) { - listeners = this._listeners = [] as Listener[]; - } + if (listeners === undefined) { + listeners = this._listeners = [] as Listener[]; + } + + if (this._inheritedEnd > 0) { + listeners.splice(0, this._inheritedEnd); + this._inheritedEnd = 0; + } - for (let i = 0; i < parentListeners.length; i++) { - let listener = parentListeners[i]; - let index = indexOfListener(listeners, listener.event, listener.target, listener.method); + for (let i = 0; i < parentListeners.length; i++) { + let listener = parentListeners[i]; + let index = indexOfListener(listeners, listener.event, listener.target, listener.method); - if (index === -1) { - listeners.unshift(listener); - this._inheritedEnd++; + if (index === -1) { + listeners.unshift(listener); + this._inheritedEnd++; + } } } } - this._inheritedVersion = currentListenerVersion; + this._setFlattened(); } return this._listeners; @@ -598,6 +640,8 @@ export class Meta { for (let index = 0; index < listeners.length; index++) { let listener = listeners[index]; + // REMOVE and REMOVE_ALL listeners are placeholders that tell us not to + // inherit, so they never match. Only ADD and ONCE can match. if ( listener.event === eventName && (listener.kind === ListenerKind.ADD || listener.kind === ListenerKind.ONCE) diff --git a/packages/@ember/-internals/meta/tests/meta_test.js b/packages/@ember/-internals/meta/tests/meta_test.js index b05c181fa69..717318bcb59 100644 --- a/packages/@ember/-internals/meta/tests/meta_test.js +++ b/packages/@ember/-internals/meta/tests/meta_test.js @@ -1,5 +1,5 @@ import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; -import { meta } from '..'; +import { meta, counters } from '..'; moduleFor( 'Ember.meta', @@ -76,6 +76,59 @@ moduleFor( assert.equal(matching[0], t); } + ['@test meta.listeners reopen after flatten'](assert) { + // Ensure counter is zeroed + counters.reopensAfterFlatten = 0; + + class Class1 {} + let class1Meta = meta(Class1.prototype); + class1Meta.addToListeners('hello', null, 'm', 0); + + let instance1 = new Class1(); + let m1 = meta(instance1); + + class Class2 {} + let class2Meta = meta(Class2.prototype); + class2Meta.addToListeners('hello', null, 'm', 0); + + let instance2 = new Class2(); + let m2 = meta(instance2); + + m1.matchingListeners('hello'); + m2.matchingListeners('hello'); + + assert.equal(counters.reopensAfterFlatten, 0, 'no reopen calls yet'); + + m1.addToListeners('world', null, 'm', 0); + m2.addToListeners('world', null, 'm', 0); + m1.matchingListeners('world'); + m2.matchingListeners('world'); + + assert.equal( + counters.reopensAfterFlatten, + 0, + 'no reopen calls after mutating leaf listeners' + ); + + class1Meta.removeFromListeners('hello', null, 'm'); + class2Meta.removeFromListeners('hello', null, 'm'); + m1.matchingListeners('hello'); + m2.matchingListeners('hello'); + + assert.equal(counters.reopensAfterFlatten, 1, 'one reopen call after mutating parents'); + + class1Meta.addToListeners('hello', null, 'm', 0); + m1.matchingListeners('hello'); + class2Meta.addToListeners('hello', null, 'm', 0); + m2.matchingListeners('hello'); + + assert.equal( + counters.reopensAfterFlatten, + 2, + 'one reopen call after mutating parents and flattening out of order' + ); + } + ['@test meta.writeWatching issues useful error after destroy']() { let target = { toString() { diff --git a/yarn.lock b/yarn.lock index f2280866ef7..bd7f793ed61 100644 --- a/yarn.lock +++ b/yarn.lock @@ -112,6 +112,10 @@ version "0.0.38" resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.38.tgz#c1be40aa933723c608820a99a373a16d215a1ca2" +"@types/node@^10.5.5": + version "10.9.3" + resolved "https://registry.yarnpkg.com/@types/node/-/node-10.9.3.tgz#85f288502503ade0b3bfc049fe1777b05d0327d5" + "@types/node@^9.6.0": version "9.6.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-9.6.0.tgz#d3480ee666df9784b1001a1872a2f6ccefb6c2d7" @@ -6401,9 +6405,11 @@ route-recognizer@^0.3.3: version "0.3.3" resolved "https://registry.yarnpkg.com/route-recognizer/-/route-recognizer-0.3.3.tgz#1d365e27fa6995e091675f7dc940a8c00353bd29" -router_js@2.0.0-beta.4: - version "2.0.0-beta.4" - resolved "https://registry.yarnpkg.com/router_js/-/router_js-2.0.0-beta.4.tgz#b61821c9f6da70e3c6114a09ef8135b389b7d0d0" +router_js@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/router_js/-/router_js-3.0.0.tgz#fcbc928bfc918d9bf1ca6fc02632f2a7cf62e0c1" + dependencies: + "@types/node" "^10.5.5" rsvp@3.0.14: version "3.0.14"