From 192167365643d8e0f340e13dfdd3eeace7950cc9 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 12 Jun 2019 18:22:31 -0400 Subject: [PATCH] [BUGFIX] Use WeakMap to store tracked property values. Using a `Symbol` causes issues on IE11 when using the core-js polyfill. Specifically, the polyfill adds every symbol to `Object.prototype` and therefore our `if (symbol in this) {` check was completely borked. Migrating to a `WeakMap` exposed another issue: we were accidentally relying on the fact that the symbols would be inherited through the prototype chain. This meant that setting a tracked property on a prototype would actually affect the value seen by every instance of that prototype. For example: ```js class Thing { @tracked baz; } Thing.prototype.baz = 'foo'; let thing = new Thing(); console.log(thing.baz) ``` Before the changes in this commit, the above console.log would emit `foo` and after these changes it would print `undefined`. --- .../@ember/-internals/metal/lib/tracked.ts | 25 ++++++++++--------- .../controller/tests/controller_test.js | 4 +-- .../tests/routing/decoupled_basic_test.js | 25 +++++++++++-------- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index a6a6fc11bb6..1b30b9b2769 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -1,4 +1,4 @@ -import { HAS_NATIVE_SYMBOL, isEmberArray, symbol as emberSymbol } from '@ember/-internals/utils'; +import { isEmberArray } from '@ember/-internals/utils'; import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; @@ -9,10 +9,6 @@ import { markObjectAsDirty, tagForProperty, update } from './tags'; type Option = T | null; -// For some reason TS can't infer that these two functions are compatible-ish, -// so we need to corece the type -let symbol = (HAS_NATIVE_SYMBOL ? Symbol : emberSymbol) as (debugKey: string) => string; - /** An object that that tracks @tracked properties that were consumed. @@ -192,7 +188,8 @@ function descriptorForField([_target, key, desc]: [ ); let initializer = desc ? desc.initializer : undefined; - let secretKey = symbol(key); + let values = new WeakMap(); + let hasInitializer = typeof initializer === 'function'; return { enumerable: true, @@ -203,12 +200,16 @@ function descriptorForField([_target, key, desc]: [ if (CURRENT_TRACKER) CURRENT_TRACKER.add(propertyTag); + let value; + // If the field has never been initialized, we should initialize it - if (!(secretKey in this)) { - this[secretKey] = typeof initializer === 'function' ? initializer.call(this) : undefined; - } + if (hasInitializer && !values.has(this)) { + value = initializer.call(this); - let value = this[secretKey]; + values.set(this, value); + } else { + value = values.get(this); + } // Add the tag of the returned value if it is an array, since arrays // should always cause updates if they are consumed and then changed @@ -216,13 +217,13 @@ function descriptorForField([_target, key, desc]: [ update(propertyTag, tagForProperty(value, '[]')); } - return this[secretKey]; + return value; }, set(newValue: any): void { markObjectAsDirty(this, key); - this[secretKey] = newValue; + values.set(this, newValue); if (propertyDidChange !== null) { propertyDidChange(); diff --git a/packages/@ember/controller/tests/controller_test.js b/packages/@ember/controller/tests/controller_test.js index 669247ec7c4..97f23d3085a 100644 --- a/packages/@ember/controller/tests/controller_test.js +++ b/packages/@ember/controller/tests/controller_test.js @@ -138,10 +138,10 @@ moduleFor( assert.expect(3); expectNoDeprecation(); - let controller = Controller.extend({ + let controller = Controller.create({ content: 'foo-bar', model: 'blammo', - }).create(); + }); assert.equal(get(controller, 'content'), 'foo-bar'); assert.equal(get(controller, 'model'), 'blammo'); diff --git a/packages/ember/tests/routing/decoupled_basic_test.js b/packages/ember/tests/routing/decoupled_basic_test.js index 7c42be46fb4..1ccb27bfb62 100644 --- a/packages/ember/tests/routing/decoupled_basic_test.js +++ b/packages/ember/tests/routing/decoupled_basic_test.js @@ -155,8 +155,9 @@ moduleFor( this.add( 'controller:homepage', Controller.extend({ - model: { - home: 'Comes from homepage', + init() { + this._super(...arguments); + this.model = { home: 'Comes from homepage' }; }, }) ); @@ -183,16 +184,18 @@ moduleFor( this.add( 'controller:foo', Controller.extend({ - model: { - home: 'Comes from foo', + init() { + this._super(...arguments); + this.model = { home: 'Comes from foo' }; }, }) ); this.add( 'controller:homepage', Controller.extend({ - model: { - home: 'Comes from homepage', + init() { + this._super(...arguments); + this.model = { home: 'Comes from homepage' }; }, }) ); @@ -220,8 +223,9 @@ moduleFor( this.add( 'controller:home', Controller.extend({ - model: { - home: 'Comes from home.', + init() { + this._super(...arguments); + this.model = { home: 'Comes from home.' }; }, }) ); @@ -241,8 +245,9 @@ moduleFor( this.add( 'controller:home', Controller.extend({ - model: { - home: 'YES I AM HOME', + init() { + this._super(...arguments); + this.model = { home: 'YES I AM HOME' }; }, }) );