From ca4d8edc989599b0e78e79ce18bd3b4a825d4ceb Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 6 Sep 2016 22:41:00 -0700 Subject: [PATCH 1/3] Call super with the right arguments --- packages/glimmer-runtime/tests/ember-component-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/glimmer-runtime/tests/ember-component-test.ts b/packages/glimmer-runtime/tests/ember-component-test.ts index b75b410194..1a931b70d6 100644 --- a/packages/glimmer-runtime/tests/ember-component-test.ts +++ b/packages/glimmer-runtime/tests/ember-component-test.ts @@ -1059,8 +1059,8 @@ QUnit.test('dynamic attribute bindings', assert => { attributeBindings = ['style']; style: string = null; - constructor() { - super(); + constructor(attrs) { + super(attrs); this.style = 'color: red;'; fooBarInstance = this; } From 172383ec9a9ed49474e2ff8a1e16f7df7b8688b3 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 6 Sep 2016 22:47:56 -0700 Subject: [PATCH 2/3] Always fire update hooks This is to align the semantics with https://github.com/emberjs/ember.js/issues/13947 Currently these things are tested in Ember, but we need to port those tests over soon. --- .../lib/compiled/opcodes/component.ts | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/glimmer-runtime/lib/compiled/opcodes/component.ts b/packages/glimmer-runtime/lib/compiled/opcodes/component.ts index 91f5cf6ee6..b8864515d1 100644 --- a/packages/glimmer-runtime/lib/compiled/opcodes/component.ts +++ b/packages/glimmer-runtime/lib/compiled/opcodes/component.ts @@ -6,7 +6,7 @@ import { CompiledArgs, EvaluatedArgs } from '../../compiled/expressions/args'; import { Templates } from '../../syntax/core'; import { DynamicScope } from '../../environment'; import Bounds from '../../bounds'; -import { CONSTANT_TAG, PathReference, ReferenceCache, Revision, combine, isConst } from 'glimmer-reference'; +import { CONSTANT_TAG, PathReference, ReferenceCache, combine, isConst } from 'glimmer-reference'; import { FIXME } from 'glimmer-util'; export class PutDynamicComponentDefinitionOpcode extends Opcode { @@ -78,8 +78,6 @@ export class OpenComponentOpcode extends Opcode { export class UpdateComponentOpcode extends UpdatingOpcode { public type = "update-component"; - private lastUpdated: Revision; - constructor( private name: string, private component: Component, @@ -89,26 +87,20 @@ export class UpdateComponentOpcode extends UpdatingOpcode { ) { super(); - let tag; let componentTag = manager.getTag(component); if (componentTag) { - tag = this.tag = combine([args.tag, componentTag]); + this.tag = combine([args.tag, componentTag]); } else { - tag = this.tag = args.tag; + this.tag = args.tag; } - - this.lastUpdated = tag.value(); } evaluate(vm: UpdatingVM) { - let { component, manager, tag, args, dynamicScope, lastUpdated } = this; + let { component, manager, args, dynamicScope } = this; - if (!tag.validate(lastUpdated)) { - manager.update(component, args, dynamicScope); - vm.env.didUpdate(component, manager); - this.lastUpdated = tag.value(); - } + manager.update(component, args, dynamicScope); + vm.env.didUpdate(component, manager); } toJSON(): OpcodeJSON { From fb5c83ee644abd2d60451ce5a42416f7a1321124 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 6 Sep 2016 23:11:16 -0700 Subject: [PATCH 3/3] Make `didCreate` and `didUpdate` post-order See https://github.com/emberjs/ember.js/issues/13972 --- .../glimmer-runtime/lib/compiled/opcodes/component.ts | 9 ++++++--- packages/glimmer-runtime/lib/environment.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/glimmer-runtime/lib/compiled/opcodes/component.ts b/packages/glimmer-runtime/lib/compiled/opcodes/component.ts index b8864515d1..ec9a52d839 100644 --- a/packages/glimmer-runtime/lib/compiled/opcodes/component.ts +++ b/packages/glimmer-runtime/lib/compiled/opcodes/component.ts @@ -69,7 +69,6 @@ export class OpenComponentOpcode extends Opcode { vm.stack().pushSimpleBlock(); vm.pushRootScope(selfRef, layout.symbols); vm.invokeLayout(args, layout, templates, callerScope, component, manager, shadow); - vm.env.didCreate(component, manager); vm.updateWith(new UpdateComponentOpcode(definition.name, component, manager, args, dynamicScope)); } @@ -100,7 +99,6 @@ export class UpdateComponentOpcode extends UpdatingOpcode { let { component, manager, args, dynamicScope } = this; manager.update(component, args, dynamicScope); - vm.env.didUpdate(component, manager); } toJSON(): OpcodeJSON { @@ -167,6 +165,8 @@ export class DidRenderLayoutOpcode extends Opcode { manager.didRenderLayout(component, bounds); + vm.env.didCreate(component, manager); + vm.updateWith(new DidUpdateLayoutOpcode(manager, component, bounds)); } } @@ -183,9 +183,12 @@ export class DidUpdateLayoutOpcode extends UpdatingOpcode { super(); } - evaluate() { + evaluate(vm: UpdatingVM) { let { manager, component, bounds } = this; + manager.didUpdateLayout(component, bounds); + + vm.env.didUpdate(component, manager); } } diff --git a/packages/glimmer-runtime/lib/environment.ts b/packages/glimmer-runtime/lib/environment.ts index 9f6f3e31d6..3d4d04344a 100644 --- a/packages/glimmer-runtime/lib/environment.ts +++ b/packages/glimmer-runtime/lib/environment.ts @@ -207,7 +207,7 @@ export abstract class Environment { manager.didCreate(component); } - for (let i=this.updatedComponents.length-1; i>=0; i--) { + for (let i=0; i