Makes modifiers and didCreate/didUpdate cache driven

This PR finishes up the autotracking refactors that were done previously, making after-render effects that used to be handled by the render transaction handled instead by an EffectsManager. The EffectsManager essentially manages a number of caches, which it keeps in lists internally. Every time a render completes, these queues are scheduled to run, with every cache in the queue being checked in order to see if something has changed. If it has, the effect runs its update. The biggest change with this refactor is that the `didCreate`/`didUpdate` hooks used by classic components will now interleave with modifiers. Previously, they always ran _before_ all modifiers, regardless of whether the modifiers were children of a given component. In theory, this shouldn't be an issue, but if it is we can separate out the component hooks into a separate queue and restore the prevous ordering.
glimmerjs · May 19, 2021 · 8e1229f · 8e1229f
1 parent 15bfb9d
commit 8e1229f
Show file tree

Hide file tree

Showing 25 changed files with 262 additions and 372 deletions.
diff --git a/packages/@glimmer/benchmark-env/src/benchmark/create-env-delegate.ts b/packages/@glimmer/benchmark-env/src/benchmark/create-env-delegate.ts
@@ -110,6 +110,9 @@ export default function createEnvDelegate(isInteractive: boolean): EnvironmentDe
   return {
     isInteractive,
     enableDebugTooling: false,
+    scheduleEffects(_phase, callback) {
+      callback();
+    },
     onTransactionCommit() {
       flush(scheduledDestructors);
       flush(scheduledFinalizers);

diff --git a/packages/@glimmer/benchmark-env/src/benchmark/on-modifier.ts b/packages/@glimmer/benchmark-env/src/benchmark/on-modifier.ts
@@ -1,7 +1,6 @@
 import { CapturedArguments, InternalModifierManager, Owner } from '@glimmer/interfaces';
 import { Reference, valueForRef } from '@glimmer/reference';
 import { castToBrowser } from '@glimmer/util';
-import { createUpdatableTag } from '@glimmer/validator';
 import { SimpleElement } from '@simple-dom/interface';
 
 interface OnModifierState {
@@ -50,10 +49,6 @@ class OnModifierManager implements InternalModifierManager<OnModifierState, obje
   getDestroyable(state: OnModifierState) {
     return state;
   }
-
-  getTag() {
-    return createUpdatableTag();
-  }
 }
 
 const onModifier: InternalModifierManager<unknown, object> = new OnModifierManager();

diff --git a/packages/@glimmer/debug/lib/opcode-metadata.ts b/packages/@glimmer/debug/lib/opcode-metadata.ts
@@ -1267,16 +1267,6 @@ METADATA[Op.CommitComponentTransaction] = {
   mnemonic: 'comp_commit',
   before: null,
   stackChange: 0,
-  ops: [],
-  operands: 0,
-  check: true,
-};
-
-METADATA[Op.DidCreateElement] = {
-  name: 'DidCreateElement',
-  mnemonic: 'comp_created',
-  before: null,
-  stackChange: 0,
   ops: [
     {
       name: 'state',
@@ -1287,9 +1277,9 @@ METADATA[Op.DidCreateElement] = {
   check: true,
 };
 
-METADATA[Op.DidRenderLayout] = {
-  name: 'DidRenderLayout',
-  mnemonic: 'comp_rendered',
+METADATA[Op.DidCreateElement] = {
+  name: 'DidCreateElement',
+  mnemonic: 'comp_created',
   before: null,
   stackChange: 0,
   ops: [

diff --git a/packages/@glimmer/integration-tests/lib/modes/env.ts b/packages/@glimmer/integration-tests/lib/modes/env.ts
@@ -192,6 +192,10 @@ export const BaseEnv: EnvironmentDelegate = {
 
   enableDebugTooling: false,
 
+  scheduleEffects(_phase, callback) {
+    callback();
+  },
+
   onTransactionCommit() {
     for (let i = 0; i < scheduledDestroyables.length; i++) {
       scheduledDestructors[i](scheduledDestroyables[i]);

diff --git a/packages/@glimmer/integration-tests/lib/modifiers.ts b/packages/@glimmer/integration-tests/lib/modifiers.ts
@@ -7,7 +7,6 @@ import {
   CapturedArguments,
   Owner,
 } from '@glimmer/interfaces';
-import { UpdatableTag, createUpdatableTag } from '@glimmer/validator';
 import { registerDestructor } from '@glimmer/destroyable';
 import { reifyPositional, reifyNamed } from '@glimmer/runtime';
 
@@ -38,10 +37,6 @@ export class TestModifierManager
     return new TestModifier(element, instance, args);
   }
 
-  getTag({ tag }: TestModifier): UpdatableTag {
-    return tag;
-  }
-
   getDebugName() {
     return '<unknown>';
   }
@@ -77,8 +72,6 @@ export class TestModifierManager
 }
 
 export class TestModifier {
-  public tag = createUpdatableTag();
-
   constructor(
     public element: SimpleElement,
     public instance: TestModifierInstance | undefined,

diff --git a/packages/@glimmer/integration-tests/test/env-test.ts b/packages/@glimmer/integration-tests/test/env-test.ts
@@ -8,7 +8,13 @@ QUnit.test('assert against nested transactions', (assert) => {
     { document: castToSimple(document) },
     {
       onTransactionCommit() {},
+
+      scheduleEffects(_phase, callback) {
+        callback();
+      },
+
       isInteractive: true,
+
       enableDebugTooling: false,
     }
   );
@@ -24,7 +30,13 @@ QUnit.test('ensure commit cleans up when it can', (assert) => {
     { document: castToSimple(document) },
     {
       onTransactionCommit() {},
+
+      scheduleEffects(_phase, callback) {
+        callback();
+      },
+
       isInteractive: true,
+
       enableDebugTooling: false,
     }
   );

diff --git a/packages/@glimmer/interfaces/index.d.ts b/packages/@glimmer/interfaces/index.d.ts
@@ -2,6 +2,7 @@ export * from './lib/core';
 export * from './lib/compile';
 export * from './lib/components';
 export * from './lib/curry';
+export * from './lib/effects';
 export * from './lib/managers';
 export * from './lib/content';
 export * from './lib/array';

diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts
@@ -8,10 +8,12 @@ import {
 } from '@simple-dom/interface';
 import { Option, Maybe } from '../core';
 import { Bounds, Cursor } from './bounds';
-import { ElementOperations, Environment, ModifierInstance } from '../runtime';
+import { ElementOperations, Environment } from '../runtime';
 import { GlimmerTreeConstruction, GlimmerTreeChanges } from './changes';
 import { Stack } from '../stack';
-import { InternalModifierManager } from '../managers';
+
+// eslint-disable-next-line node/no-extraneous-import
+import { Cache } from '@glimmer/validator';
 
 export interface LiveBlock extends Bounds {
   openElement(element: SimpleElement): void;
@@ -42,7 +44,7 @@ export interface DOMStack {
   popRemoteElement(): void;
   popElement(): void;
   openElement(tag: string, _operations?: ElementOperations): SimpleElement;
-  flushElement(modifiers: Option<ModifierInstance[]>): void;
+  flushElement(modifiers: Option<Cache[]>): void;
   appendText(string: string): SimpleText;
   appendComment(string: string): SimpleComment;
 
@@ -59,7 +61,7 @@ export interface DOMStack {
     namespace: Option<string>
   ): AttributeOperation;
 
-  closeElement(): Option<ModifierInstance[]>;
+  closeElement(): Option<Cache[]>;
 }
 
 export interface TreeOperations {

diff --git a/packages/@glimmer/interfaces/lib/effects.d.ts b/packages/@glimmer/interfaces/lib/effects.d.ts
@@ -0,0 +1,6 @@
+// eslint-disable-next-line node/no-extraneous-import
+import { Cache } from '@glimmer/validator';
+
+export const enum EffectPhase {
+  Layout = 'layout',
+}
diff --git a/packages/@glimmer/interfaces/lib/managers/internal/modifier.d.ts b/packages/@glimmer/interfaces/lib/managers/internal/modifier.d.ts
@@ -19,10 +19,6 @@ export interface InternalModifierManager<
     args: CapturedArguments
   ): TModifierInstanceState;
 
-  // Convert the opaque modifier into a `RevisionTag` that determins when
-  // the modifier's update hooks need to be called (if at all).
-  getTag(modifier: TModifierInstanceState): UpdatableTag | null;
-
   getDebugName(Modifier: TModifierDefinitionState): string;
 
   // At initial render, the modifier gets a chance to install itself on the

diff --git a/packages/@glimmer/interfaces/lib/runtime/environment.d.ts b/packages/@glimmer/interfaces/lib/runtime/environment.d.ts
@@ -1,37 +1,24 @@
 import { SimpleDocument } from '@simple-dom/interface';
-import { ComponentDefinitionState, ComponentInstance, ComponentInstanceState } from '../components';
-import { Option } from '../core';
 import { GlimmerTreeChanges, GlimmerTreeConstruction } from '../dom/changes';
 import { DebugRenderTree } from './debug-render-tree';
-import { Owner } from './owner';
-import { ModifierInstance } from './modifier';
-import { WithCreateInstance } from '../..';
+import { EffectPhase } from '../..';
+
+// eslint-disable-next-line node/no-extraneous-import
+import { Cache } from '@glimmer/validator';
 
 export interface EnvironmentOptions {
   document?: SimpleDocument;
   appendOperations?: GlimmerTreeConstruction;
   updateOperations?: GlimmerTreeChanges;
 }
 
-export interface Transaction {}
-
 declare const TransactionSymbol: unique symbol;
 export type TransactionSymbol = typeof TransactionSymbol;
 
-export type ComponentInstanceWithCreate = ComponentInstance<
-  ComponentDefinitionState,
-  ComponentInstanceState,
-  WithCreateInstance
->;
-
 export interface Environment {
-  [TransactionSymbol]: Option<Transaction>;
-
-  didCreate(component: ComponentInstanceWithCreate): void;
-  didUpdate(component: ComponentInstanceWithCreate): void;
+  [TransactionSymbol]: boolean;
 
-  scheduleInstallModifier(modifier: ModifierInstance): void;
-  scheduleUpdateModifier(modifier: ModifierInstance): void;
+  registerEffect(phase: EffectPhase, cache: Cache): void;
 
   begin(): void;
   commit(): void;

diff --git a/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts b/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts
@@ -96,17 +96,16 @@ export const enum Op {
   BeginComponentTransaction = 97,
   CommitComponentTransaction = 98,
   DidCreateElement = 99,
-  DidRenderLayout = 100,
-  InvokePartial = 101,
-  ResolveMaybeLocal = 102,
-  Debugger = 103,
-  Size = 104,
-  StaticComponentAttr = 105,
-  DynamicContentType = 106,
-  DynamicHelper = 107,
-  DynamicModifier = 108,
-  IfInline = 109,
-  Not = 110,
-  GetDynamicVar = 111,
-  Log = 112,
+  InvokePartial = 100,
+  ResolveMaybeLocal = 101,
+  Debugger = 102,
+  Size = 103,
+  StaticComponentAttr = 104,
+  DynamicContentType = 105,
+  DynamicHelper = 106,
+  DynamicModifier = 107,
+  IfInline = 108,
+  Not = 109,
+  GetDynamicVar = 110,
+  Log = 111,
 }
diff --git a/packages/@glimmer/manager/lib/public/modifier.ts b/packages/@glimmer/manager/lib/public/modifier.ts
@@ -178,10 +178,6 @@ export class CustomModifierManager<O extends Owner, ModifierInstance>
     return debugName!;
   }
 
-  getTag({ tag }: CustomModifierState<ModifierInstance>) {
-    return tag;
-  }
-
   install({ element, args, modifier, delegate }: CustomModifierState<ModifierInstance>) {
     let { capabilities } = delegate;
 

diff --git a/packages/@glimmer/manager/test/managers-test.ts b/packages/@glimmer/manager/test/managers-test.ts
@@ -7,7 +7,6 @@ import {
   ModifierManager,
 } from '@glimmer/interfaces';
 import { UNDEFINED_REFERENCE } from '@glimmer/reference';
-import { createUpdatableTag } from '@glimmer/validator';
 
 import {
   setInternalComponentManager,
@@ -296,10 +295,6 @@ module('Managers', () => {
           return null;
         }
 
-        getTag() {
-          return createUpdatableTag();
-        }
-
         install() {}
 
         update() {}

diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts
@@ -1,14 +1,8 @@
-import type {
-  Bounds,
-  Environment,
-  Option,
-  ElementBuilder,
-  Maybe,
-  ModifierInstance,
-} from '@glimmer/interfaces';
+import type { Bounds, Environment, Option, ElementBuilder, Maybe } from '@glimmer/interfaces';
 import { ConcreteBounds, NewElementBuilder } from '@glimmer/runtime';
 import { RemoteLiveBlock } from '@glimmer/runtime';
 import type { SimpleElement, SimpleNode, SimpleText } from '@simple-dom/interface';
+import { Cache } from '@glimmer/validator';
 
 const TEXT_NODE = 3;
 
@@ -94,7 +88,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder {
     return super.__appendText(string);
   }
 
-  closeElement(): Option<ModifierInstance[]> {
+  closeElement(): Option<Cache[]> {
     if (NEEDS_EXTRA_CLOSE.has(this.element)) {
       NEEDS_EXTRA_CLOSE.delete(this.element);
       super.closeElement();

diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts
@@ -314,7 +314,7 @@ function InvokeStaticComponent(
   op(Op.Constant, layoutOperand(layout));
   op(Op.CompileBlock);
   op(MachineOp.InvokeVirtual);
-  op(Op.DidRenderLayout, $s0);
+  op(Op.CommitComponentTransaction, $s0);
 
   op(MachineOp.PopFrame);
   op(Op.PopScope);
@@ -323,7 +323,6 @@ function InvokeStaticComponent(
     op(Op.PopDynamicScope);
   }
 
-  op(Op.CommitComponentTransaction);
   op(Op.Load, $s0);
 }
 
@@ -422,12 +421,11 @@ export function invokePreparedComponent(
 
   op(Op.Pop, 1);
   op(Op.InvokeComponentLayout, $s0);
-  op(Op.DidRenderLayout, $s0);
-  op(MachineOp.PopFrame);
+  op(Op.CommitComponentTransaction, $s0);
 
+  op(MachineOp.PopFrame);
   op(Op.PopScope);
   op(Op.PopDynamicScope);
-  op(Op.CommitComponentTransaction);
 }
 
 export function InvokeBareComponent(op: PushStatementOp): void {