From 5a5d0d9dc029e21cfaed7a16d000a05fc902b8d7 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Wed, 19 May 2021 13:45:50 -0700 Subject: [PATCH 1/2] 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. --- .../src/benchmark/create-env-delegate.ts | 3 + .../src/benchmark/on-modifier.ts | 5 - .../@glimmer/debug/lib/opcode-metadata.ts | 16 +- .../integration-tests/lib/modes/env.ts | 4 + .../integration-tests/lib/modifiers.ts | 7 - .../integration-tests/test/env-test.ts | 29 ++-- packages/@glimmer/interfaces/index.d.ts | 1 + .../interfaces/lib/dom/attributes.d.ts | 10 +- packages/@glimmer/interfaces/lib/effects.d.ts | 6 + .../lib/managers/internal/modifier.d.ts | 4 - .../interfaces/lib/runtime/environment.d.ts | 25 +-- .../@glimmer/interfaces/lib/vm-opcodes.d.ts | 25 ++- .../@glimmer/manager/lib/public/modifier.ts | 4 - .../@glimmer/manager/test/managers-test.ts | 5 - .../@glimmer/node/lib/serialize-builder.ts | 12 +- .../lib/opcode-builder/helpers/components.ts | 8 +- .../runtime/lib/compiled/opcodes/component.ts | 62 ++++---- .../runtime/lib/compiled/opcodes/dom.ts | 147 +++++------------- packages/@glimmer/runtime/lib/effects.ts | 88 +++++++++++ packages/@glimmer/runtime/lib/environment.ts | 146 +++-------------- packages/@glimmer/runtime/lib/modifiers/on.ts | 10 -- packages/@glimmer/runtime/lib/vm/append.ts | 5 +- .../runtime/lib/vm/element-builder.ts | 12 +- packages/@glimmer/validator/lib/tracking.ts | 2 +- packages/@glimmer/vm/lib/opcodes.toml | 7 +- 25 files changed, 266 insertions(+), 377 deletions(-) create mode 100644 packages/@glimmer/interfaces/lib/effects.d.ts create mode 100644 packages/@glimmer/runtime/lib/effects.ts diff --git a/packages/@glimmer/benchmark-env/src/benchmark/create-env-delegate.ts b/packages/@glimmer/benchmark-env/src/benchmark/create-env-delegate.ts index 2b9c7f0bd8..356d85f547 100644 --- 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 index 07d7799f05..70d7a3729b 100644 --- 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 = new OnModifierManager(); diff --git a/packages/@glimmer/debug/lib/opcode-metadata.ts b/packages/@glimmer/debug/lib/opcode-metadata.ts index 3760b1245f..9a282691f6 100644 --- 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 index 555751f36d..3b83684d39 100644 --- 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 index dc2f9069ca..d238c463d1 100644 --- 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 ''; } @@ -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 index 5400db14b2..b95792f238 100644 --- 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, } ); @@ -23,24 +29,21 @@ QUnit.test('ensure commit cleans up when it can', (assert) => { let env = new EnvironmentImpl( { document: castToSimple(document) }, { - onTransactionCommit() {}, + onTransactionCommit() { + throw new Error('something failed'); + }, + + scheduleEffects(_phase, callback) { + callback(); + }, + isInteractive: true, + enableDebugTooling: false, } ); - env.begin(); - // ghetto stub - Object.defineProperty(env, 'transaction', { - get() { - return { - scheduledInstallManagers(): void {}, - commit(): void { - throw new Error('something failed'); - }, - }; - }, - }); + env.begin(); assert.throws(() => env.commit(), 'something failed'); // commit failed diff --git a/packages/@glimmer/interfaces/index.d.ts b/packages/@glimmer/interfaces/index.d.ts index 7af0424c9b..36e0ae7836 100644 --- 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 index 3b9214839b..9feb05aa4a 100644 --- 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): void; + flushElement(modifiers: Option): void; appendText(string: string): SimpleText; appendComment(string: string): SimpleComment; @@ -59,7 +61,7 @@ export interface DOMStack { namespace: Option ): AttributeOperation; - closeElement(): Option; + closeElement(): Option; } export interface TreeOperations { diff --git a/packages/@glimmer/interfaces/lib/effects.d.ts b/packages/@glimmer/interfaces/lib/effects.d.ts new file mode 100644 index 0000000000..4e39a504a9 --- /dev/null +++ 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 index 13778e26db..ceb44712bc 100644 --- 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 index fc8d33f9b3..c0b2a700e7 100644 --- a/packages/@glimmer/interfaces/lib/runtime/environment.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/environment.d.ts @@ -1,11 +1,10 @@ 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; @@ -13,25 +12,13 @@ export interface EnvironmentOptions { 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; - - 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 index 31e028aee9..efe7ce5eb7 100644 --- 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 index 18c35539c7..d53a7fd0dd 100644 --- a/packages/@glimmer/manager/lib/public/modifier.ts +++ b/packages/@glimmer/manager/lib/public/modifier.ts @@ -178,10 +178,6 @@ export class CustomModifierManager return debugName!; } - getTag({ tag }: CustomModifierState) { - return tag; - } - install({ element, args, modifier, delegate }: CustomModifierState) { let { capabilities } = delegate; diff --git a/packages/@glimmer/manager/test/managers-test.ts b/packages/@glimmer/manager/test/managers-test.ts index 84b24ff0d7..37e6a0a89c 100644 --- 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 index ffd2c85929..84a5aa8101 100644 --- 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 { + closeElement(): Option { 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 index f4dd8199de..3f62e16bb6 100644 --- 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 { diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index e638a31291..fb70ad3133 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -31,12 +31,12 @@ import { CapturedArguments, CompilableProgram, ComponentInstance, - ModifierInstance, - ComponentInstanceWithCreate, Owner, CurriedType, UpdatingOpcode, + EffectPhase, } from '@glimmer/interfaces'; +import { Cache, consumeTag, createCache, untrack } from '@glimmer/validator'; import { isConstRef, Reference, valueForRef } from '@glimmer/reference'; import { assert, @@ -460,7 +460,7 @@ type DeferredAttribute = { export class ComponentElementOperations implements ElementOperations { private attributes = dict(); private classes: (string | Reference)[] = []; - private modifiers: ModifierInstance[] = []; + private modifiers: Cache[] = []; setAttribute( name: string, @@ -487,11 +487,11 @@ export class ComponentElementOperations implements ElementOperations { this.attributes[name] = deferred; } - addModifier(modifier: ModifierInstance): void { + addModifier(modifier: Cache): void { this.modifiers.push(modifier); } - flush(vm: InternalVM): ModifierInstance[] { + flush(vm: InternalVM): Cache[] { let type: DeferredAttribute | undefined; let attributes = this.attributes; @@ -826,7 +826,7 @@ APPEND_OPCODES.add(Op.InvokeComponentLayout, (vm, { op1: _state }) => { vm.call(state.handle!); }); -APPEND_OPCODES.add(Op.DidRenderLayout, (vm, { op1: _state }) => { +APPEND_OPCODES.add(Op.CommitComponentTransaction, (vm, { op1: _state }) => { let instance = check(vm.fetchValue(_state), CheckComponentInstance); let { manager, state, capabilities } = instance; let bounds = vm.elements().popBlock(); @@ -849,17 +849,40 @@ APPEND_OPCODES.add(Op.DidRenderLayout, (vm, { op1: _state }) => { } } + let tag = vm.commitCacheGroup(); + if (managerHasCapability(manager, capabilities, InternalComponentCapability.CreateInstance)) { - let mgr = check(manager, CheckInterface({ didRenderLayout: CheckFunction })); + let mgr = check( + manager, + CheckInterface({ + didRenderLayout: CheckFunction, + didUpdateLayout: CheckFunction, + didCreate: CheckFunction, + didUpdate: CheckFunction, + }) + ); mgr.didRenderLayout(state, bounds); - vm.env.didCreate(instance as ComponentInstanceWithCreate); - vm.updateWith(new DidUpdateLayoutOpcode(instance as ComponentInstanceWithCreate, bounds)); - } -}); + let didCreate = false; + + let cache = createCache(() => { + consumeTag(tag); + + untrack(() => { + if (didCreate === false) { + didCreate = true; + + mgr.didCreate(state); + } else { + mgr.didUpdateLayout(state, bounds); + mgr.didUpdate(state); + } + }); + }); -APPEND_OPCODES.add(Op.CommitComponentTransaction, (vm) => { - vm.commitCacheGroup(); + vm.env.registerEffect(EffectPhase.Layout, cache); + vm.associateDestroyable(cache); + } }); export class UpdateComponentOpcode implements UpdatingOpcode { @@ -876,19 +899,6 @@ export class UpdateComponentOpcode implements UpdatingOpcode { } } -export class DidUpdateLayoutOpcode implements UpdatingOpcode { - constructor(private component: ComponentInstanceWithCreate, private bounds: Bounds) {} - - evaluate(vm: UpdatingVM) { - let { component, bounds } = this; - let { manager, state } = component; - - manager.didUpdateLayout(state, bounds); - - vm.env.didUpdate(component); - } -} - class DebugRenderTreeUpdateOpcode implements UpdatingOpcode { constructor(private bucket: object) {} diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index 92ddfaf89e..29109c55a0 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -1,12 +1,5 @@ import { Reference, valueForRef, isConstRef, createComputeRef } from '@glimmer/reference'; -import { - Revision, - Tag, - valueForTag, - validateTag, - consumeTag, - CURRENT_TAG, -} from '@glimmer/validator'; +import { Cache, createCache, getValue } from '@glimmer/validator'; import { check, CheckString, @@ -25,8 +18,8 @@ import { CurriedType, ModifierDefinitionState, Environment, - UpdatingVM, UpdatingOpcode, + EffectPhase, } from '@glimmer/interfaces'; import { $t0 } from '@glimmer/vm'; import { APPEND_OPCODES } from '../../opcodes'; @@ -37,7 +30,7 @@ import { CONSTANTS } from '../../symbols'; import { assign, debugToString, expect, isObject } from '@glimmer/util'; import { CurriedValue, isCurriedType, resolveCurriedValue } from '../../curried-value'; import { DEBUG } from '@glimmer/env'; -import { associateDestroyableChild, destroy } from '@glimmer/destroyable'; +import { associateDestroyableChild, destroy, isDestroying } from '@glimmer/destroyable'; APPEND_OPCODES.add(Op.Text, (vm, { op1: text }) => { vm.elements().appendText(vm[CONSTANTS].getValue(text)); @@ -83,7 +76,7 @@ APPEND_OPCODES.add(Op.PopRemoteElement, (vm) => { APPEND_OPCODES.add(Op.FlushElement, (vm) => { let operations = check(vm.fetchValue($t0), CheckOperations); - let modifiers: Option = null; + let modifiers: Option = null; if (operations) { modifiers = operations.flush(vm); @@ -98,13 +91,8 @@ APPEND_OPCODES.add(Op.CloseElement, (vm) => { if (modifiers) { modifiers.forEach((modifier) => { - vm.env.scheduleInstallModifier(modifier); - let { manager, state } = modifier; - let d = manager.getDestroyable(state); - - if (d) { - vm.associateDestroyable(d); - } + vm.env.registerEffect(EffectPhase.Layout, modifier); + vm.associateDestroyable(modifier); }); } }); @@ -129,25 +117,32 @@ APPEND_OPCODES.add(Op.Modifier, (vm, { op1: handle }) => { args.capture() ); - let instance: ModifierInstance = { - manager, - state, - definition, - }; - let operations = expect( check(vm.fetchValue($t0), CheckOperations), 'BUG: ElementModifier could not find operations to append to' ); - operations.addModifier(instance); + let didSetup = false; + + let cache = createCache(() => { + if (isDestroying(cache)) return; - let tag = manager.getTag(state); + if (didSetup === false) { + didSetup = true; - if (tag !== null) { - consumeTag(tag); - return vm.updateWith(new UpdateModifierOpcode(tag, instance)); + manager.install(state); + } else { + manager.update(state); + } + }, DEBUG && `- While rendering:\n (instance of a \`${definition.resolvedName || manager.getDebugName(definition.state)}\` modifier)`); + + let d = manager.getDestroyable(state); + + if (d) { + associateDestroyableChild(cache, d); } + + operations.addModifier(cache); }); APPEND_OPCODES.add(Op.DynamicModifier, (vm) => { @@ -161,7 +156,7 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => { let { constructing } = vm.elements(); let initialOwner = vm.getOwner(); - let instanceRef = createComputeRef(() => { + let instanceCache = createCache(() => { let value = valueForRef(ref); let owner: Owner; @@ -226,65 +221,14 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => { }; }); - let instance = valueForRef(instanceRef); - let tag = null; - - if (instance !== undefined) { - let operations = expect( - check(vm.fetchValue($t0), CheckOperations), - 'BUG: ElementModifier could not find operations to append to' - ); - - operations.addModifier(instance); - - tag = instance.manager.getTag(instance.state); - - if (tag !== null) { - consumeTag(tag); - } - } - - if (!isConstRef(ref) || tag) { - return vm.updateWith(new UpdateDynamicModifierOpcode(tag, instance, instanceRef)); - } -}); - -export class UpdateModifierOpcode implements UpdatingOpcode { - private lastUpdated: Revision; - - constructor(private tag: Tag, private modifier: ModifierInstance) { - this.lastUpdated = valueForTag(tag); - } - - evaluate(vm: UpdatingVM) { - let { modifier, tag, lastUpdated } = this; - - consumeTag(tag); + let instance: ModifierInstance | undefined; - if (!validateTag(tag, lastUpdated)) { - vm.env.scheduleUpdateModifier(modifier); - this.lastUpdated = valueForTag(tag); - } - } -} + let cache = createCache(() => { + if (isDestroying(cache)) return; -export class UpdateDynamicModifierOpcode implements UpdatingOpcode { - private lastUpdated: Revision; + let newInstance = getValue(instanceCache); - constructor( - private tag: Tag | null, - private instance: ModifierInstance | undefined, - private instanceRef: Reference - ) { - this.lastUpdated = valueForTag(tag ?? CURRENT_TAG); - } - - evaluate(vm: UpdatingVM) { - let { tag, lastUpdated, instance, instanceRef } = this; - - let newInstance = valueForRef(instanceRef); - - if (newInstance !== instance) { + if (instance !== newInstance) { if (instance !== undefined) { let destroyable = instance.manager.getDestroyable(instance.state); @@ -298,30 +242,25 @@ export class UpdateDynamicModifierOpcode implements UpdatingOpcode { let destroyable = manager.getDestroyable(state); if (destroyable !== null) { - associateDestroyableChild(this, destroyable); - } - - tag = manager.getTag(state); - - if (tag !== null) { - this.lastUpdated = valueForTag(tag); + associateDestroyableChild(cache, destroyable); } - this.tag = tag; - vm.env.scheduleInstallModifier(newInstance!); + manager.install(newInstance.state); } - this.instance = newInstance; - } else if (tag !== null && !validateTag(tag, lastUpdated)) { - vm.env.scheduleUpdateModifier(instance!); - this.lastUpdated = valueForTag(tag); + instance = newInstance; + } else if (instance !== undefined) { + instance.manager.update(instance.state); } + }, DEBUG && `- While rendering:\n (instance of a dynamic modifier)`); - if (tag !== null) { - consumeTag(tag); - } - } -} + let operations = expect( + check(vm.fetchValue($t0), CheckOperations), + 'BUG: ElementModifier could not find operations to append to' + ); + + operations.addModifier(cache); +}); APPEND_OPCODES.add(Op.StaticAttr, (vm, { op1: _name, op2: _value, op3: _namespace }) => { let name = vm[CONSTANTS].getValue(_name); diff --git a/packages/@glimmer/runtime/lib/effects.ts b/packages/@glimmer/runtime/lib/effects.ts new file mode 100644 index 0000000000..e3dad437fd --- /dev/null +++ b/packages/@glimmer/runtime/lib/effects.ts @@ -0,0 +1,88 @@ +import { EffectPhase } from '@glimmer/interfaces'; +import { assert } from '@glimmer/util'; +import { Cache, getValue } from '@glimmer/validator'; +import { DEBUG } from '@glimmer/env'; +import { registerDestructor } from '@glimmer/destroyable'; + +// Use this to get all the effect phases into a tuple in a typesafe way +// values are unimportant, but key order is important is it's the order that +// the phases should logically run in +let phases: { [key in EffectPhase]: unknown } = { + [EffectPhase.Layout]: 0, +}; + +const EFFECT_PHASES: EffectPhase[] = Object.keys(phases) as EffectPhase[]; + +export class EffectsManager { + private inTransaction = false; + + constructor(private scheduleEffects: (phase: EffectPhase, callback: () => void) => void) {} + + private effects: { [key in EffectPhase]: Cache[] } = { + [EffectPhase.Layout]: [], + }; + + /** + * Tracker for new effects added within a given render transaction. This is + * used to coordinate adding effects to the queue. In a given render pass, all + * effects added should be added in the order they were received, but they + * should be _prepended_ to any pre-existing effects. For instance, let's say + * that the queue started off in this state after our first render pass: + * + * A, B, C + * + * On the next render pass, we add D and E, which are siblings, and children + * of A. The timing semantics of effects is that siblings should be + * initialized in the order they were defined in, and should run before + * parents. So, assuming D comes before E, we want to do: + * + * D, E, A, B, C + * + * This way, new children will always run in the correct order, and before + * their parents. By keeping track of the new effects during a given + * transaction in a separate array, we can then efficiently add them to the + * beginning of the overall effect queue at the end, and preserve the proper + * order. + */ + private newEffects: { [key in EffectPhase]: Cache[] } = { + [EffectPhase.Layout]: [], + }; + + begin() { + if (DEBUG) { + this.inTransaction = true; + } + } + + registerEffect(phase: EffectPhase, effect: Cache) { + assert(this.inTransaction, 'You cannot register effects unless you are in a transaction'); + + this.newEffects[phase].push(effect); + + registerDestructor(effect, () => { + let queue = this.effects[phase]; + let index = queue.indexOf(effect); + + assert(index !== -1, 'attempted to remove an effect, but it was not in the effect queue'); + + queue.splice(index, 1); + }); + } + + commit() { + if (DEBUG) { + this.inTransaction = false; + } + + let { effects, newEffects, scheduleEffects } = this; + + for (let phase of EFFECT_PHASES) { + let queue = newEffects[phase].concat(effects[phase]); + effects[phase] = queue; + newEffects[phase] = []; + + // weirdness here to avoid closure assertion in Ember + scheduleEffects(phase, queue.forEach.bind(queue, getValue)); + } + } +} diff --git a/packages/@glimmer/runtime/lib/environment.ts b/packages/@glimmer/runtime/lib/environment.ts index ad455a16e2..a84fcfd565 100644 --- a/packages/@glimmer/runtime/lib/environment.ts +++ b/packages/@glimmer/runtime/lib/environment.ts @@ -4,114 +4,23 @@ import { EnvironmentOptions, GlimmerTreeChanges, GlimmerTreeConstruction, - Transaction, TransactionSymbol, RuntimeContext, RuntimeResolver, - Option, RuntimeArtifacts, - ComponentInstanceWithCreate, - ModifierInstance, - InternalModifierManager, - ModifierInstanceState, + EffectPhase, } from '@glimmer/interfaces'; import { assert, expect, symbol } from '@glimmer/util'; -import { track, updateTag } from '@glimmer/validator'; +import { Cache } from '@glimmer/validator'; import { DOMChangesImpl, DOMTreeConstruction } from './dom/helper'; import { RuntimeProgramImpl } from '@glimmer/program'; import DebugRenderTree from './debug-render-tree'; +import { EffectsManager } from './effects'; export const TRANSACTION: TransactionSymbol = symbol('TRANSACTION'); -class TransactionImpl implements Transaction { - public scheduledInstallModifiers: ModifierInstance[] = []; - public scheduledUpdateModifiers: ModifierInstance[] = []; - public createdComponents: ComponentInstanceWithCreate[] = []; - public updatedComponents: ComponentInstanceWithCreate[] = []; - - didCreate(component: ComponentInstanceWithCreate) { - this.createdComponents.push(component); - } - - didUpdate(component: ComponentInstanceWithCreate) { - this.updatedComponents.push(component); - } - - scheduleInstallModifier(modifier: ModifierInstance) { - this.scheduledInstallModifiers.push(modifier); - } - - scheduleUpdateModifier(modifier: ModifierInstance) { - this.scheduledUpdateModifiers.push(modifier); - } - - commit() { - let { createdComponents, updatedComponents } = this; - - for (let i = 0; i < createdComponents.length; i++) { - let { manager, state } = createdComponents[i]; - manager.didCreate(state); - } - - for (let i = 0; i < updatedComponents.length; i++) { - let { manager, state } = updatedComponents[i]; - manager.didUpdate(state); - } - - let { scheduledInstallModifiers, scheduledUpdateModifiers } = this; - - // Prevent a transpilation issue we guard against in Ember, the - // throw-if-closure-required issue - let manager: InternalModifierManager, state: ModifierInstanceState; - - for (let i = 0; i < scheduledInstallModifiers.length; i++) { - let modifier = scheduledInstallModifiers[i]; - manager = modifier.manager; - state = modifier.state; - - let modifierTag = manager.getTag(state); - - if (modifierTag !== null) { - let tag = track( - // eslint-disable-next-line no-loop-func - () => manager.install(state), - DEBUG && - `- While rendering:\n (instance of a \`${ - modifier.definition.resolvedName || manager.getDebugName(modifier.definition.state) - }\` modifier)` - ); - updateTag(modifierTag, tag); - } else { - manager.install(state); - } - } - - for (let i = 0; i < scheduledUpdateModifiers.length; i++) { - let modifier = scheduledUpdateModifiers[i]; - manager = modifier.manager; - state = modifier.state; - - let modifierTag = manager.getTag(state); - - if (modifierTag !== null) { - let tag = track( - // eslint-disable-next-line no-loop-func - () => manager.update(state), - DEBUG && - `- While rendering:\n (instance of a \`${ - modifier.definition.resolvedName || manager.getDebugName(modifier.definition.state) - }\` modifier)` - ); - updateTag(modifierTag, tag); - } else { - manager.update(state); - } - } - } -} - export class EnvironmentImpl implements Environment { - [TRANSACTION]: Option = null; + [TRANSACTION] = false; protected appendOperations!: GlimmerTreeConstruction; protected updateOperations?: GlimmerTreeChanges; @@ -121,6 +30,8 @@ export class EnvironmentImpl implements Environment { debugRenderTree = this.delegate.enableDebugTooling ? new DebugRenderTree() : undefined; + private effectManager = new EffectsManager(this.delegate.scheduleEffects); + constructor(options: EnvironmentOptions, private delegate: EnvironmentDelegate) { if (options.appendOperations) { this.appendOperations = options.appendOperations; @@ -150,39 +61,21 @@ export class EnvironmentImpl implements Environment { 'A glimmer transaction was begun, but one already exists. You may have a nested transaction, possibly caused by an earlier runtime exception while rendering. Please check your console for the stack trace of any prior exceptions.' ); - this.debugRenderTree?.begin(); - - this[TRANSACTION] = new TransactionImpl(); - } - - private get transaction(): TransactionImpl { - return expect(this[TRANSACTION]!, 'must be in a transaction'); - } - - didCreate(component: ComponentInstanceWithCreate) { - this.transaction.didCreate(component); - } + this.effectManager.begin(); - didUpdate(component: ComponentInstanceWithCreate) { - this.transaction.didUpdate(component); - } + this.debugRenderTree?.begin(); - scheduleInstallModifier(modifier: ModifierInstance) { - if (this.isInteractive) { - this.transaction.scheduleInstallModifier(modifier); - } + this[TRANSACTION] = true; } - scheduleUpdateModifier(modifier: ModifierInstance) { - if (this.isInteractive) { - this.transaction.scheduleUpdateModifier(modifier); - } + registerEffect(phase: EffectPhase, effect: Cache) { + this.effectManager.registerEffect(phase, effect); } commit() { - let transaction = this.transaction; - this[TRANSACTION] = null; - transaction.commit(); + this[TRANSACTION] = false; + + this.effectManager.commit(); this.debugRenderTree?.commit(); @@ -202,6 +95,17 @@ export interface EnvironmentDelegate { */ enableDebugTooling: boolean; + /** + * Allows the embedding environment to schedule effects to be run in the future. + * Different phases will be passed to this callback, and each one should be + * scheduled at an appropriate time for that phase. The callback will be + * called at the end each transaction. + * + * @param phase the phase of effects that are being scheduled + * @param runEffects the callback which runs the effects + */ + scheduleEffects: (phase: EffectPhase, runEffects: () => void) => void; + /** * Callback to be called when an environment transaction commits */ diff --git a/packages/@glimmer/runtime/lib/modifiers/on.ts b/packages/@glimmer/runtime/lib/modifiers/on.ts index 8b15f3916e..cd41aa8985 100644 --- a/packages/@glimmer/runtime/lib/modifiers/on.ts +++ b/packages/@glimmer/runtime/lib/modifiers/on.ts @@ -4,7 +4,6 @@ import { CapturedArguments, InternalModifierManager, Owner } from '@glimmer/inte import { setInternalModifierManager } from '@glimmer/manager'; import { valueForRef } from '@glimmer/reference'; import { reifyNamed } from '@glimmer/runtime'; -import { createUpdatableTag, UpdatableTag } from '@glimmer/validator'; import { SimpleElement } from '@simple-dom/interface'; import { buildUntouchableThis } from '@glimmer/util'; @@ -46,7 +45,6 @@ const SUPPORTS_EVENT_OPTIONS = (() => { })(); export class OnModifierState { - public tag = createUpdatableTag(); public element: Element; public args: CapturedArguments; public eventName!: string; @@ -329,14 +327,6 @@ class OnModifierManager implements InternalModifierManager(); - private modifierStack = new Stack>(); + private modifierStack = new Stack>(); private blockStack = new Stack(); static forInitialRender(env: Environment, cursor: CursorImpl) { @@ -182,7 +182,7 @@ export class NewElementBuilder implements ElementBuilder { return this.dom.createElement(tag, this.element); } - flushElement(modifiers: Option) { + flushElement(modifiers: Option) { let parent = this.element; let element = expect( this.constructing, @@ -203,7 +203,7 @@ export class NewElementBuilder implements ElementBuilder { this.dom.insertBefore(parent, constructing, this.nextSibling); } - closeElement(): Option { + closeElement(): Option { this.willCloseElement(); this.popElement(); return this.popModifiers(); @@ -244,11 +244,11 @@ export class NewElementBuilder implements ElementBuilder { this[CURSOR_STACK].push(new CursorImpl(element, nextSibling)); } - private pushModifiers(modifiers: Option): void { + private pushModifiers(modifiers: Option): void { this.modifierStack.push(modifiers); } - private popModifiers(): Option { + private popModifiers(): Option { return this.modifierStack.pop(); } diff --git a/packages/@glimmer/validator/lib/tracking.ts b/packages/@glimmer/validator/lib/tracking.ts index 4e8e001119..c1ddb47795 100644 --- a/packages/@glimmer/validator/lib/tracking.ts +++ b/packages/@glimmer/validator/lib/tracking.ts @@ -182,7 +182,7 @@ export function getValue(cache: Cache): T | undefined { let snapshot = cache[SNAPSHOT]; if (tag === undefined || !validateTag(tag, snapshot)) { - beginTrackFrame(); + beginTrackFrame(DEBUG && cache[DEBUG_LABEL]); try { cache[LAST_VALUE] = fn(); diff --git a/packages/@glimmer/vm/lib/opcodes.toml b/packages/@glimmer/vm/lib/opcodes.toml index 4b3cb63641..0de154544b 100644 --- a/packages/@glimmer/vm/lib/opcodes.toml +++ b/packages/@glimmer/vm/lib/opcodes.toml @@ -872,7 +872,7 @@ operand-stack = [ [syscall.comp_commit] -format = "CommitComponentTransaction" +format = ["CommitComponentTransaction", "state:register"] operation = "Commit the current cache group" [syscall.comp_created] @@ -880,11 +880,6 @@ operation = "Commit the current cache group" format = ["DidCreateElement", "state:register"] operation = "Invoke didCreateElement on the current component manager" -[syscall.comp_rendered] - -format = ["DidRenderLayout", "state:register"] -operation = "Invoke didRenderLayout on the current component manager" - [syscall.invokepartial] format = ["InvokePartial", "owner:owner", "symbols:str-array", "evalInfo:array"] From ecef385b2b954b4e5224a94b8e5ec8778e888e11 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 23 May 2021 10:11:05 -0700 Subject: [PATCH 2/2] try reference --- .../interfaces/lib/dom/attributes.d.ts | 6 +- .../interfaces/lib/runtime/environment.d.ts | 4 +- .../@glimmer/node/lib/serialize-builder.ts | 4 +- .../runtime/lib/compiled/opcodes/component.ts | 12 +-- .../runtime/lib/compiled/opcodes/dom.ts | 76 +++++++++++-------- packages/@glimmer/runtime/lib/effects.ts | 10 +-- packages/@glimmer/runtime/lib/environment.ts | 4 +- .../runtime/lib/vm/element-builder.ts | 12 +-- 8 files changed, 69 insertions(+), 59 deletions(-) diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts index 9feb05aa4a..3a5c7b0edb 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -13,7 +13,7 @@ import { GlimmerTreeConstruction, GlimmerTreeChanges } from './changes'; import { Stack } from '../stack'; // eslint-disable-next-line node/no-extraneous-import -import { Cache } from '@glimmer/validator'; +import { Reference } from '@glimmer/reference'; export interface LiveBlock extends Bounds { openElement(element: SimpleElement): void; @@ -44,7 +44,7 @@ export interface DOMStack { popRemoteElement(): void; popElement(): void; openElement(tag: string, _operations?: ElementOperations): SimpleElement; - flushElement(modifiers: Option): void; + flushElement(modifiers: Option): void; appendText(string: string): SimpleText; appendComment(string: string): SimpleComment; @@ -61,7 +61,7 @@ export interface DOMStack { namespace: Option ): AttributeOperation; - closeElement(): Option; + closeElement(): Option; } export interface TreeOperations { diff --git a/packages/@glimmer/interfaces/lib/runtime/environment.d.ts b/packages/@glimmer/interfaces/lib/runtime/environment.d.ts index c0b2a700e7..49f08896ff 100644 --- a/packages/@glimmer/interfaces/lib/runtime/environment.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/environment.d.ts @@ -4,7 +4,7 @@ import { DebugRenderTree } from './debug-render-tree'; import { EffectPhase } from '../..'; // eslint-disable-next-line node/no-extraneous-import -import { Cache } from '@glimmer/validator'; +import { Reference } from '@glimmer/reference'; export interface EnvironmentOptions { document?: SimpleDocument; @@ -18,7 +18,7 @@ export type TransactionSymbol = typeof TransactionSymbol; export interface Environment { [TransactionSymbol]: boolean; - registerEffect(phase: EffectPhase, cache: Cache): void; + registerEffect(phase: EffectPhase, cache: Reference): void; begin(): void; commit(): void; diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts index 84a5aa8101..6081f8d84c 100644 --- a/packages/@glimmer/node/lib/serialize-builder.ts +++ b/packages/@glimmer/node/lib/serialize-builder.ts @@ -2,7 +2,7 @@ import type { Bounds, Environment, Option, ElementBuilder, Maybe } from '@glimme 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'; +import { Reference } from '@glimmer/reference'; const TEXT_NODE = 3; @@ -88,7 +88,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder { return super.__appendText(string); } - closeElement(): Option { + closeElement(): Option { if (NEEDS_EXTRA_CLOSE.has(this.element)) { NEEDS_EXTRA_CLOSE.delete(this.element); super.closeElement(); diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index fb70ad3133..139c249ce6 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -36,8 +36,8 @@ import { UpdatingOpcode, EffectPhase, } from '@glimmer/interfaces'; -import { Cache, consumeTag, createCache, untrack } from '@glimmer/validator'; -import { isConstRef, Reference, valueForRef } from '@glimmer/reference'; +import { consumeTag, untrack } from '@glimmer/validator'; +import { createComputeRef, isConstRef, Reference, valueForRef } from '@glimmer/reference'; import { assert, assign, @@ -460,7 +460,7 @@ type DeferredAttribute = { export class ComponentElementOperations implements ElementOperations { private attributes = dict(); private classes: (string | Reference)[] = []; - private modifiers: Cache[] = []; + private modifiers: Reference[] = []; setAttribute( name: string, @@ -487,11 +487,11 @@ export class ComponentElementOperations implements ElementOperations { this.attributes[name] = deferred; } - addModifier(modifier: Cache): void { + addModifier(modifier: Reference): void { this.modifiers.push(modifier); } - flush(vm: InternalVM): Cache[] { + flush(vm: InternalVM): Reference[] { let type: DeferredAttribute | undefined; let attributes = this.attributes; @@ -865,7 +865,7 @@ APPEND_OPCODES.add(Op.CommitComponentTransaction, (vm, { op1: _state }) => { let didCreate = false; - let cache = createCache(() => { + let cache = createComputeRef(() => { consumeTag(tag); untrack(() => { diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index 29109c55a0..090868ed26 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -1,5 +1,4 @@ import { Reference, valueForRef, isConstRef, createComputeRef } from '@glimmer/reference'; -import { Cache, createCache, getValue } from '@glimmer/validator'; import { check, CheckString, @@ -76,7 +75,7 @@ APPEND_OPCODES.add(Op.PopRemoteElement, (vm) => { APPEND_OPCODES.add(Op.FlushElement, (vm) => { let operations = check(vm.fetchValue($t0), CheckOperations); - let modifiers: Option = null; + let modifiers: Option = null; if (operations) { modifiers = operations.flush(vm); @@ -124,17 +123,24 @@ APPEND_OPCODES.add(Op.Modifier, (vm, { op1: handle }) => { let didSetup = false; - let cache = createCache(() => { - if (isDestroying(cache)) return; + let cache = createComputeRef( + () => { + if (isDestroying(cache)) return; - if (didSetup === false) { - didSetup = true; + if (didSetup === false) { + didSetup = true; - manager.install(state); - } else { - manager.update(state); - } - }, DEBUG && `- While rendering:\n (instance of a \`${definition.resolvedName || manager.getDebugName(definition.state)}\` modifier)`); + manager.install(state); + } else { + manager.update(state); + } + }, + null, + DEBUG && + `- While rendering:\n (instance of a \`${ + definition.resolvedName || manager.getDebugName(definition.state) + }\` modifier)` + ); let d = manager.getDestroyable(state); @@ -156,7 +162,7 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => { let { constructing } = vm.elements(); let initialOwner = vm.getOwner(); - let instanceCache = createCache(() => { + let instanceCache = createComputeRef(() => { let value = valueForRef(ref); let owner: Owner; @@ -223,36 +229,40 @@ APPEND_OPCODES.add(Op.DynamicModifier, (vm) => { let instance: ModifierInstance | undefined; - let cache = createCache(() => { - if (isDestroying(cache)) return; + let cache = createComputeRef( + () => { + if (isDestroying(cache)) return; - let newInstance = getValue(instanceCache); + let newInstance = valueForRef(instanceCache); - if (instance !== newInstance) { - if (instance !== undefined) { - let destroyable = instance.manager.getDestroyable(instance.state); + if (instance !== newInstance) { + if (instance !== undefined) { + let destroyable = instance.manager.getDestroyable(instance.state); - if (destroyable !== null) { - destroy(destroyable); + if (destroyable !== null) { + destroy(destroyable); + } } - } - if (newInstance !== undefined) { - let { manager, state } = newInstance; - let destroyable = manager.getDestroyable(state); + if (newInstance !== undefined) { + let { manager, state } = newInstance; + let destroyable = manager.getDestroyable(state); + + if (destroyable !== null) { + associateDestroyableChild(cache, destroyable); + } - if (destroyable !== null) { - associateDestroyableChild(cache, destroyable); + manager.install(newInstance.state); } - manager.install(newInstance.state); + instance = newInstance; + } else if (instance !== undefined) { + instance.manager.update(instance.state); } - - instance = newInstance; - } else if (instance !== undefined) { - instance.manager.update(instance.state); - } - }, DEBUG && `- While rendering:\n (instance of a dynamic modifier)`); + }, + null, + DEBUG && `- While rendering:\n (instance of a dynamic modifier)` + ); let operations = expect( check(vm.fetchValue($t0), CheckOperations), diff --git a/packages/@glimmer/runtime/lib/effects.ts b/packages/@glimmer/runtime/lib/effects.ts index e3dad437fd..429a4c1e4b 100644 --- a/packages/@glimmer/runtime/lib/effects.ts +++ b/packages/@glimmer/runtime/lib/effects.ts @@ -1,6 +1,6 @@ import { EffectPhase } from '@glimmer/interfaces'; import { assert } from '@glimmer/util'; -import { Cache, getValue } from '@glimmer/validator'; +import { Reference, valueForRef } from '@glimmer/reference'; import { DEBUG } from '@glimmer/env'; import { registerDestructor } from '@glimmer/destroyable'; @@ -18,7 +18,7 @@ export class EffectsManager { constructor(private scheduleEffects: (phase: EffectPhase, callback: () => void) => void) {} - private effects: { [key in EffectPhase]: Cache[] } = { + private effects: { [key in EffectPhase]: Reference[] } = { [EffectPhase.Layout]: [], }; @@ -44,7 +44,7 @@ export class EffectsManager { * beginning of the overall effect queue at the end, and preserve the proper * order. */ - private newEffects: { [key in EffectPhase]: Cache[] } = { + private newEffects: { [key in EffectPhase]: Reference[] } = { [EffectPhase.Layout]: [], }; @@ -54,7 +54,7 @@ export class EffectsManager { } } - registerEffect(phase: EffectPhase, effect: Cache) { + registerEffect(phase: EffectPhase, effect: Reference) { assert(this.inTransaction, 'You cannot register effects unless you are in a transaction'); this.newEffects[phase].push(effect); @@ -82,7 +82,7 @@ export class EffectsManager { newEffects[phase] = []; // weirdness here to avoid closure assertion in Ember - scheduleEffects(phase, queue.forEach.bind(queue, getValue)); + scheduleEffects(phase, queue.forEach.bind(queue, valueForRef)); } } } diff --git a/packages/@glimmer/runtime/lib/environment.ts b/packages/@glimmer/runtime/lib/environment.ts index a84fcfd565..4a15fb7061 100644 --- a/packages/@glimmer/runtime/lib/environment.ts +++ b/packages/@glimmer/runtime/lib/environment.ts @@ -11,11 +11,11 @@ import { EffectPhase, } from '@glimmer/interfaces'; import { assert, expect, symbol } from '@glimmer/util'; -import { Cache } from '@glimmer/validator'; import { DOMChangesImpl, DOMTreeConstruction } from './dom/helper'; import { RuntimeProgramImpl } from '@glimmer/program'; import DebugRenderTree from './debug-render-tree'; import { EffectsManager } from './effects'; +import { Reference } from '@glimmer/reference'; export const TRANSACTION: TransactionSymbol = symbol('TRANSACTION'); @@ -68,7 +68,7 @@ export class EnvironmentImpl implements Environment { this[TRANSACTION] = true; } - registerEffect(phase: EffectPhase, effect: Cache) { + registerEffect(phase: EffectPhase, effect: Reference) { this.effectManager.registerEffect(phase, effect); } diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index f490d4610d..3c7372ce89 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -23,8 +23,8 @@ import { } from '@simple-dom/interface'; import { clear, ConcreteBounds, CursorImpl, SingleNodeBounds } from '../bounds'; import { destroy, registerDestructor } from '@glimmer/destroyable'; -import { Cache } from '@glimmer/validator'; import { DynamicAttribute, dynamicAttribute } from './attributes/dynamic'; +import { Reference } from '@glimmer/reference'; export interface FirstNode { firstNode(): SimpleNode; @@ -80,7 +80,7 @@ export class NewElementBuilder implements ElementBuilder { private env: Environment; [CURSOR_STACK] = new Stack(); - private modifierStack = new Stack>(); + private modifierStack = new Stack>(); private blockStack = new Stack(); static forInitialRender(env: Environment, cursor: CursorImpl) { @@ -182,7 +182,7 @@ export class NewElementBuilder implements ElementBuilder { return this.dom.createElement(tag, this.element); } - flushElement(modifiers: Option) { + flushElement(modifiers: Option) { let parent = this.element; let element = expect( this.constructing, @@ -203,7 +203,7 @@ export class NewElementBuilder implements ElementBuilder { this.dom.insertBefore(parent, constructing, this.nextSibling); } - closeElement(): Option { + closeElement(): Option { this.willCloseElement(); this.popElement(); return this.popModifiers(); @@ -244,11 +244,11 @@ export class NewElementBuilder implements ElementBuilder { this[CURSOR_STACK].push(new CursorImpl(element, nextSibling)); } - private pushModifiers(modifiers: Option): void { + private pushModifiers(modifiers: Option): void { this.modifierStack.push(modifiers); } - private popModifiers(): Option { + private popModifiers(): Option { return this.modifierStack.pop(); }