[BUGFIX] Updates the assignment of args to reflect the arg proxy

The refactor to use a proxy for tracking individual updates to args requires us to make some changes here in how we assign arguments. Also adds the `updateHook` option to capabilities. This solution should be compatible with Ember pre and post enabling the tracked canary flag, but can be made better with ember-compatibility-helpers once we have a version of Ember to target.
glimmerjs · Aug 8, 2019 · 82f4863 · 82f4863
1 parent b1a84c0
commit 82f4863
Show file tree

Hide file tree

Showing 8 changed files with 441 additions and 114 deletions.
diff --git a/packages/@glimmer/component/addon/-private/component-manager.ts b/packages/@glimmer/component/addon/-private/component-manager.ts
@@ -1,11 +1,12 @@
 import { DEBUG } from '@glimmer/env';
 import Ember from 'ember';
+import { set } from '@ember/object';
 import { getOwner, setOwner } from '@ember/application';
 import ApplicationInstance from '@ember/application/instance';
 import { capabilities } from '@ember/component';
 import { schedule } from '@ember/runloop';
 
-import GlimmerComponent, { DESTROYING, DESTROYED, MAGIC_PROP } from './component';
+import GlimmerComponent, { DESTROYING, DESTROYED, ARGS_SET } from './component';
 
 export interface ComponentManagerArgs {
   named: object;
@@ -26,29 +27,19 @@ export default class GlimmerComponentManager {
     this.capabilities = capabilities('3.4', {
       destructor: true,
       asyncLifecycleCallbacks: true,
+      updateHook: false,
     });
   }
 
   createComponent(
     Klass: typeof GlimmerComponent,
     args: ComponentManagerArgs
   ): CreateComponentResult {
-    let instance;
-
-    let argSnapshot = args.named;
-
     if (DEBUG) {
-      argSnapshot = Object.assign({}, argSnapshot);
-      Object.defineProperty(argSnapshot, MAGIC_PROP, {
-        enumerable: false,
-        value: true,
-      });
-      argSnapshot = Object.freeze(argSnapshot);
+      ARGS_SET.add(args.named);
     }
 
-    instance = new Klass(getOwner(this), argSnapshot);
-
-    return instance as CreateComponentResult;
+    return new Klass(getOwner(this), args.named) as CreateComponentResult;
   }
 
   updateComponent(component: CreateComponentResult, args: ComponentManagerArgs) {
@@ -58,7 +49,7 @@ export default class GlimmerComponentManager {
       argSnapshot = Object.freeze(argSnapshot);
     }
 
-    component.args = argSnapshot;
+    set(component, 'args', argSnapshot);
   }
 
   destroyComponent(component: CreateComponentResult) {

diff --git a/packages/@glimmer/component/addon/-private/component.ts b/packages/@glimmer/component/addon/-private/component.ts
@@ -4,13 +4,13 @@ import { setOwner } from './owner';
 const DESTROYING = Symbol('destroying');
 const DESTROYED = Symbol('destroyed');
 
-let MAGIC_PROP: symbol;
+let ARGS_SET: WeakSet<any>;
 
 if (DEBUG) {
-  MAGIC_PROP = Symbol('magic-prop');
+  ARGS_SET = new WeakSet();
 }
 
-export { DESTROYING, DESTROYED, MAGIC_PROP };
+export { DESTROYING, DESTROYED, ARGS_SET };
 
 /**
  * The `Component` class defines an encapsulated UI element that is rendered to
@@ -145,10 +145,7 @@ export default class GlimmerComponent<T = object> {
    * @param args
    */
   constructor(owner: unknown, args: T) {
-    if (
-      DEBUG &&
-      !(owner !== null && typeof owner === 'object' && (args as any)[MAGIC_PROP] === true)
-    ) {
+    if (DEBUG && !(owner !== null && typeof owner === 'object' && ARGS_SET.has(args))) {
       throw new Error(
         `You must pass both the owner and args to super() in your component: ${
           this.constructor.name

diff --git a/packages/@glimmer/component/addon/index.ts b/packages/@glimmer/component/addon/index.ts
@@ -1,20 +1,9 @@
 import ApplicationInstance from '@ember/application/instance';
 import { setComponentManager } from '@ember/component';
-import { get, set } from '@ember/object';
 import { gte } from 'ember-compatibility-helpers';
 
 import GlimmerComponentManager from './-private/component-manager';
-import _GlimmerComponent from './-private/component';
-
-class GlimmerComponent<T> extends _GlimmerComponent<T> {
-  get args(): Readonly<T> {
-    return get(this as any, '__args__');
-  }
-
-  set args(args) {
-    set(this as any, '__args__', args);
-  }
-}
+import GlimmerComponent from './-private/component';
 
 if (gte('3.8.0-beta.1')) {
   setComponentManager((owner: ApplicationInstance) => {

diff --git a/packages/@glimmer/component/package.json b/packages/@glimmer/component/package.json
@@ -32,11 +32,11 @@
     "@glimmer/env": "^0.1.7",
     "@glimmer/reference": "^0.41.0",
     "@glimmer/runtime": "^0.41.0",
-    "@glimmer/util": "^0.41.0",
     "@glimmer/tracking": "^0.14.0-alpha.9",
+    "@glimmer/util": "^0.41.0",
     "broccoli-file-creator": "^2.1.1",
     "broccoli-merge-trees": "^3.0.2",
-    "ember-cli-babel": "^7.2.0",
+    "ember-cli-babel": "^7.7.3",
     "ember-cli-get-component-path-option": "^1.0.0",
     "ember-cli-is-package-missing": "^1.0.0",
     "ember-cli-normalize-entity-name": "^1.0.0",
@@ -46,7 +46,6 @@
     "ember-compatibility-helpers": "^1.1.2"
   },
   "devDependencies": {
-    "@ember-decorators/babel-transforms": "^5.1.3",
     "@ember/optional-features": "^0.6.1",
     "@glimmer/application-test-helpers": "^0.14.0-alpha.9",
     "@glimmer/compiler": "^0.41.0",
@@ -74,7 +73,7 @@
     "ember-cli-qunit": "^4.3.2",
     "ember-cli-sri": "^2.1.0",
     "ember-cli-uglify": "^2.1.0",
-    "ember-decorators": "^5.1.3",
+    "ember-decorators-polyfill": "^1.0.6",
     "ember-disable-prototype-extensions": "^1.1.2",
     "ember-export-application-global": "^2.0.0",
     "ember-load-initializers": "^1.1.0",
@@ -98,4 +97,4 @@
     "defaultBlueprint": "install-glimmer-component",
     "main": "ember-addon-main.js"
   }
-}
+}
diff --git a/packages/@glimmer/component/src/component-manager.ts b/packages/@glimmer/component/src/component-manager.ts
@@ -29,7 +29,7 @@ import ExtendedTemplateMeta from './template-meta';
 import { SerializedTemplateWithLazyBlock } from '@glimmer/application/src/loaders/runtime-compiler/resolver';
 import { Specifier } from '@glimmer/application/src/loaders/runtime-compiler/loader';
 
-import { MAGIC_PROP, DESTROYING, DESTROYED } from '../addon/-private/component';
+import { ARGS_SET, DESTROYING, DESTROYED } from '../addon/-private/component';
 
 export interface ConstructorOptions {
   env: EnvironmentWithOwner;
@@ -67,10 +67,7 @@ export class ComponentStateBucket {
     let snapshot = this.args.named.value();
 
     if (DEBUG) {
-      Object.defineProperty(snapshot, MAGIC_PROP, {
-        enumerable: false,
-        value: true,
-      });
+      ARGS_SET.add(snapshot);
     }
 
     return Object.freeze(snapshot);

diff --git a/packages/@glimmer/component/test/ember/dummy/app/components/conference-speakers.js b/packages/@glimmer/component/test/ember/dummy/app/components/conference-speakers.js
@@ -1,6 +1,5 @@
 import Component from '@glimmer/component';
-import { action, computed } from '@ember-decorators/object';
-import { set } from '@ember/object';
+import { action, set, computed } from '@ember/object';
 
 export default class ConferenceSpeakers extends Component {
   current = 0;
@@ -13,7 +12,7 @@ export default class ConferenceSpeakers extends Component {
 
   @computed('current')
   get moreSpeakers() {
-    return (this.speakers.length - 1) > this.current;
+    return this.speakers.length - 1 > this.current;
   }
 
   @action