[FEAT] Adds Effect Queue

packages/@glimmer/integration-tests/index.ts

@@ -17,3 +17,4 @@ export * from './lib/suites';
 export * from './lib/test-helpers/module';
 export * from './lib/test-helpers/strings';
 export * from './lib/test-helpers/test';
+export * from './lib/test-helpers/tracked';

packages/@glimmer/integration-tests/lib/modifiers.ts

@@ -3,12 +3,11 @@ import {
   Dict,
   ModifierManager,
   GlimmerTreeChanges,
-  Destroyable,
   DynamicScope,
   VMArguments,
   CapturedArguments,
 } from '@glimmer/interfaces';
-import { Tag } from '@glimmer/validator';
+import { Tag, consumeTag } from '@glimmer/validator';
 
 export interface TestModifierConstructor {
   new (): TestModifierInstance;
@@ -22,12 +21,7 @@ export interface TestModifierInstance {
 }
 
 export class TestModifierDefinitionState {
-  instance?: TestModifierInstance;
-  constructor(Klass?: TestModifierConstructor) {
-    if (Klass) {
-      this.instance = new Klass();
-    }
-  }
+  constructor(public Class: TestModifierConstructor) {}
 }
 
 export class TestModifierManager
@@ -39,46 +33,48 @@ export class TestModifierManager
     _dynamicScope: DynamicScope,
     dom: GlimmerTreeChanges
   ) {
-    return new TestModifier(element, state, args.capture(), dom);
+    let { Class } = state;
+
+    return new TestModifier(element, new Class(), args.capture(), dom);
   }
 
   getTag({ args: { tag } }: TestModifier): Tag {
     return tag;
   }
 
-  install({ element, args, state }: TestModifier) {
-    if (state.instance && state.instance.didInsertElement) {
-      state.instance.element = element;
-      state.instance.didInsertElement(args.positional.value(), args.named.value());
+  install({ element, args, instance }: TestModifier) {
+    consumeTag(args.tag);
+
+    if (instance && instance.didInsertElement) {
+      instance.element = element;
+      instance.didInsertElement(args.positional.value(), args.named.value());
     }
 
     return;
   }
 
-  update({ args, state }: TestModifier) {
-    if (state.instance && state.instance.didUpdate) {
-      state.instance.didUpdate(args.positional.value(), args.named.value());
+  update({ args, instance }: TestModifier) {
+    consumeTag(args.tag);
+
+    if (instance && instance.didUpdate) {
+      instance.didUpdate(args.positional.value(), args.named.value());
     }
 
     return;
   }
 
-  getDestructor(modifier: TestModifier): Destroyable {
-    return {
-      destroy: () => {
-        let { state } = modifier;
-        if (state.instance && state.instance.willDestroyElement) {
-          state.instance.willDestroyElement();
-        }
-      },
-    };
+  teardown(modifier: TestModifier): void {
+    let { instance } = modifier;
+    if (instance && instance.willDestroyElement) {
+      instance.willDestroyElement();
+    }
   }
 }
 
 export class TestModifier {
   constructor(
     public element: SimpleElement,
-    public state: TestModifierDefinitionState,
+    public instance: TestModifierInstance,
     public args: CapturedArguments,
     public dom: GlimmerTreeChanges
   ) {}

packages/@glimmer/integration-tests/test/modifiers-test.ts

@@ -1,4 +1,4 @@
-import { RenderTest, jitSuite, test, Count } from '..';
+import { RenderTest, jitSuite, test, Count, tracked, EmberishGlimmerComponent, EmberishGlimmerArgs } from '..';
 import { Dict } from '@glimmer/interfaces';
 import { SimpleElement } from '@simple-dom/interface';
 import { Option } from '@glimmer/interfaces';
@@ -302,6 +302,72 @@ class ModifierTests extends RenderTest {
     assert.deepEqual(elementIds, ['outer-div', 'inner-div'], 'Modifiers are called on all levels');
   }
 
+  @test
+  'child modifiers that are added later update before parents'(assert: Assert) {
+    let inserts: Option<string>[] = [];
+    let updates: Option<string>[] = [];
+
+    class Bar extends BaseModifier {
+      didInsertElement(params: unknown[]) {
+        assert.deepEqual(params, [123]);
+        if (this.element) {
+          inserts.push(this.element.getAttribute('id'));
+        }
+      }
+
+      didUpdate(params: unknown[]) {
+        assert.deepEqual(params, [124]);
+        if (this.element) {
+          updates.push(this.element.getAttribute('id'));
+        }
+      }
+    }
+
+    let component: Option<Foo> = null;
+
+    class Foo extends EmberishGlimmerComponent {
+      constructor(args: EmberishGlimmerArgs) {
+        super(args);
+
+        component = this;
+      }
+
+      @tracked showing = false;
+      @tracked count = 123;
+    }
+
+    this.registerComponent(
+      'Glimmer',
+      'Foo',
+      `
+        <div id="outer" {{bar this.count}}>
+          {{#if this.showing}}
+            <div id="inner" {{bar this.count}}></div>
+          {{/if}}
+        </div>
+      `,
+      Foo
+    );
+    this.registerModifier('bar', Bar);
+
+    this.render('<Foo/>');
+
+    assert.deepEqual(inserts, ['outer'], 'outer modifier insert called');
+    assert.deepEqual(updates, [], 'no updates called');
+
+    component!.showing = true;
+    this.rerender();
+
+    assert.deepEqual(inserts, ['outer', 'inner'], 'inner modifier insert called');
+    assert.deepEqual(updates, [], 'no updates called');
+
+    component!.count++;
+    this.rerender();
+
+    assert.deepEqual(inserts, ['outer', 'inner'], 'inserts not called');
+    assert.deepEqual(updates, ['inner', 'outer'], 'updates called in correct order');
+  }
+
   @test
   'same element insertion order'(assert: Assert) {
     let insertionOrder: string[] = [];

packages/@glimmer/interfaces/lib/runtime/environment.d.ts

@@ -8,7 +8,7 @@ import { AttributeOperation } from '../dom/attributes';
 import { AttrNamespace, SimpleElement, SimpleDocument } from '@simple-dom/interface';
 import { ComponentInstanceState } from '../components';
 import { ComponentManager } from '../components/component-manager';
-import { Drop, Option } from '../core';
+import { Drop, Option, SymbolDestroyable } from '../core';
 import { GlimmerTreeChanges, GlimmerTreeConstruction } from '../dom/changes';
 import { ModifierManager } from './modifier';
 
@@ -26,6 +26,10 @@ export interface Transaction {}
 declare const TransactionSymbol: unique symbol;
 export type TransactionSymbol = typeof TransactionSymbol;
 
+export interface Effect extends SymbolDestroyable {
+  createOrUpdate(): void;
+}
+
 export interface Environment<Extra = unknown> {
   [TransactionSymbol]: Option<Transaction>;
 
@@ -35,8 +39,7 @@ export interface Environment<Extra = unknown> {
   willDestroy(drop: Drop): void;
   didDestroy(drop: Drop): void;
 
-  scheduleInstallModifier(modifier: unknown, manager: ModifierManager): void;
-  scheduleUpdateModifier(modifier: unknown, manager: ModifierManager): void;
+  registerEffect(phase: 'layout', effect: Effect): void;
 
   begin(): void;
   commit(): void;

packages/@glimmer/interfaces/lib/runtime/modifier.d.ts

@@ -34,7 +34,7 @@ export interface ModifierManager<
 
   // Convert the opaque token into an object that implements Destroyable.
   // If it returns null, the modifier will not be destroyed.
-  getDestructor(modifier: ModifierInstanceState): Option<SymbolDestroyable | Destroyable>;
+  teardown(modifier: ModifierInstanceState): void;
 }
 
 export interface ModifierDefinition<

packages/@glimmer/runtime/index.ts

@@ -25,13 +25,12 @@ export {
   isWhitespace,
 } from './lib/dom/helper';
 export { normalizeProperty } from './lib/dom/props';
-export { DefaultDynamicScope } from './lib/dynamic-scope';
+export { PartialScopeImpl, DefaultDynamicScope } from './lib/scope';
 export {
   AotRuntime,
   JitRuntime,
   EnvironmentImpl,
   EnvironmentDelegate,
-  ScopeImpl,
   JitProgramCompilationContext,
   JitSyntaxCompilationContext,
   inTransaction,

packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts

@@ -28,7 +28,7 @@ import {
 } from '@glimmer/interfaces';
 import { VersionedPathReference, Reference } from '@glimmer/reference';
 import { Tag, COMPUTE } from '@glimmer/validator';
-import { ScopeImpl } from '../../environment';
+import { PartialScopeImpl } from '../../scope';
 import CurryComponentReference from '../../references/curry-component';
 import {
   CapturedNamedArgumentsImpl,
@@ -99,7 +99,9 @@ export const CheckCapturedArguments: Checker<CapturedArguments> = CheckInterface
 
 export const CheckCurryComponent = wrap(() => CheckInstanceof(CurryComponentReference));
 
-export const CheckScope: Checker<Scope<JitOrAotBlock>> = wrap(() => CheckInstanceof(ScopeImpl));
+export const CheckScope: Checker<Scope<JitOrAotBlock>> = wrap(() =>
+  CheckInstanceof(PartialScopeImpl)
+);
 
 export const CheckComponentManager: Checker<ComponentManager<unknown>> = CheckInterface({
   getCapabilities: CheckFunction,

packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts

@@ -1,20 +1,9 @@
 import { Reference, ReferenceCache, VersionedReference } from '@glimmer/reference';
-import {
-  Revision,
-  Tag,
-  isConstTagged,
-  isConstTag,
-  valueForTag,
-  validateTag,
-} from '@glimmer/validator';
+import { Revision, Tag, isConstTagged, valueForTag, validateTag } from '@glimmer/validator';
 import { check, CheckString, CheckElement, CheckOption, CheckNode } from '@glimmer/debug';
 import { Op, Option, ModifierManager } from '@glimmer/interfaces';
 import { $t0 } from '@glimmer/vm';
-import {
-  ModifierDefinition,
-  InternalModifierManager,
-  ModifierInstanceState,
-} from '../../modifier/interfaces';
+import { ModifierDefinition } from '../../modifier/interfaces';
 import { APPEND_OPCODES, UpdatingOpcode } from '../../opcodes';
 import { UpdatingVM } from '../../vm';
 import { Assert } from './vm';
@@ -23,6 +12,7 @@ import { CheckReference, CheckArguments, CheckOperations } from './-debug-strip'
 import { CONSTANTS } from '../../symbols';
 import { SimpleElement, SimpleNode } from '@simple-dom/interface';
 import { expect, Maybe } from '@glimmer/util';
+import { EffectImpl } from '../../effects';
 
 APPEND_OPCODES.add(Op.Text, (vm, { op1: text }) => {
   vm.elements().appendText(vm[CONSTANTS].getString(text));
@@ -93,12 +83,22 @@ APPEND_OPCODES.add(Op.CloseElement, vm => {
 
   if (modifiers) {
     modifiers.forEach(([manager, modifier]) => {
-      vm.env.scheduleInstallModifier(modifier, manager);
-      let d = manager.getDestructor(modifier);
-
-      if (d) {
-        vm.associateDestroyable(d);
-      }
+      let effect = new EffectImpl({
+        setup() {
+          manager.install(modifier);
+        },
+
+        update() {
+          manager.update(modifier);
+        },
+
+        teardown() {
+          manager.teardown(modifier);
+        },
+      });
+
+      vm.env.registerEffect('layout', effect);
+      vm.associateDestroyable(effect);
     });
   }
 });
@@ -123,37 +123,8 @@ APPEND_OPCODES.add(Op.Modifier, (vm, { op1: handle }) => {
   );
 
   operations.addModifier(manager, modifier);
-
-  let tag = manager.getTag(modifier);
-
-  if (!isConstTag(tag)) {
-    vm.updateWith(new UpdateModifierOpcode(tag, manager, modifier));
-  }
 });
 
-export class UpdateModifierOpcode extends UpdatingOpcode {
-  public type = 'update-modifier';
-  private lastUpdated: Revision;
-
-  constructor(
-    public tag: Tag,
-    private manager: InternalModifierManager,
-    private modifier: ModifierInstanceState
-  ) {
-    super();
-    this.lastUpdated = valueForTag(tag);
-  }
-
-  evaluate(vm: UpdatingVM) {
-    let { manager, modifier, tag, lastUpdated } = this;
-
-    if (!validateTag(tag, lastUpdated)) {
-      vm.env.scheduleUpdateModifier(modifier, manager);
-      this.lastUpdated = valueForTag(tag);
-    }
-  }
-}
-
 APPEND_OPCODES.add(Op.StaticAttr, (vm, { op1: _name, op2: _value, op3: _namespace }) => {
   let name = vm[CONSTANTS].getString(_name);
   let value = vm[CONSTANTS].getString(_value);