From 59feb003412950e7872872870979d3b4629bf986 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 29 May 2019 03:12:01 +0200 Subject: [PATCH 1/4] Add test for non-interactive custom modifier lifecycle hooks (cherry picked from commit ff760cf345a464c9413f06cef2d82d3749077977) --- .../custom-modifier-manager-test.js | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js index 23ee4bde75a..36c2ac04b8e 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js @@ -178,3 +178,38 @@ moduleFor( } } ); + +moduleFor( + 'Rendering test: non-interactive custom modifiers', + class extends RenderingTestCase { + getBootOptions() { + return { isInteractive: false }; + } + + [`@test doesn't trigger lifecycle hooks when non-interactive`](assert) { + let ModifierClass = setModifierManager( + owner => { + return new CustomModifierManager(owner); + }, + EmberObject.extend({ + didInsertElement() { + assert.ok(false); + }, + didUpdate() { + assert.ok(false); + }, + willDestroyElement() { + assert.ok(false); + }, + }) + ); + + this.registerModifier('foo-bar', ModifierClass); + + this.render('

hello world

'); + runTask(() => this.context.set('baz', 'Hello')); + + this.assertHTML('

hello world

'); + } + } +); From c514f47427707754bfe0299af5404763ccbd6014 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 29 May 2019 13:30:59 +0200 Subject: [PATCH 2/4] Add test for non-interactive `on` modifier lifecycle hooks (cherry picked from commit 13e422a907a77d7c2f076ee4963141923edb2e7d) --- .../tests/integration/modifiers/on-test.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js b/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js index 5901ca24ff7..02d9db9b365 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/modifiers/on-test.js @@ -4,6 +4,8 @@ import { isChrome, isFirefox } from '@ember/-internals/browser-environment'; import { privatize as P } from '@ember/-internals/container'; import { HAS_NATIVE_PROXY } from '@ember/-internals/utils'; +import { Component } from '../../utils/helpers'; + const isIE11 = !window.ActiveXObject && 'ActiveXObject' in window; if (EMBER_GLIMMER_ON_MODIFIER) { @@ -379,4 +381,63 @@ if (EMBER_GLIMMER_ON_MODIFIER) { } } ); + + moduleFor( + 'Rendering test: non-interactive `on` modifier', + class extends RenderingTestCase { + getBootOptions() { + return { isInteractive: false }; + } + + beforeEach() { + // might error if getOnManagerInstance fails + this.startingCounters = this.getOnManagerInstance().counters; + } + + getOnManagerInstance() { + // leveraging private APIs, this can be deleted if these APIs change + // but it has been useful to verify some internal details + let templateCompiler = this.owner.lookup(P`template-compiler:main`); + + return templateCompiler.resolver.resolver.builtInModifiers.on.manager; + } + + assertCounts(expected) { + let { counters } = this.getOnManagerInstance(); + + this.assert.deepEqual( + counters, + { + adds: expected.adds + this.startingCounters.adds, + removes: expected.removes + this.startingCounters.removes, + }, + `counters have incremented by ${JSON.stringify(expected)}` + ); + } + + [`@test doesn't trigger lifecycle hooks when non-interactive`](assert) { + this.registerComponent('foo-bar2', { + ComponentClass: Component.extend({ + tagName: '', + fire() { + assert.ok(false); + }, + }), + template: ``, + }); + + this.render('{{#if this.showButton}}{{/if}}', { + showButton: true, + }); + this.assertHTML(''); + this.assertCounts({ adds: 0, removes: 0 }); + + this.$('button').click(); + + runTask(() => this.context.set('showButton', false)); + + this.assertCounts({ adds: 0, removes: 0 }); + } + } + ); } From a3c0b5e7d1456b9ff42492375dd7799669e53710 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 3 Jun 2019 16:48:24 -0400 Subject: [PATCH 3/4] [BUGFIX lts] Ensure custom modifier managers are not invoked in SSR. Prior to this change, custom modifier managers were invoked during non-interactive rendering invocations as well as interactive. In practice, that resulted in custom modifiers erroring whenever they ran in FastBoot / SSR environments. This change, ensures that the interactive / non-interactive state is threaded through to the required infrastructure to avoid invoking _any_ hooks on custom modifier managers when running in non-interactive modes. --- .../glimmer/lib/modifiers/custom.ts | 39 ++++++++++++++++--- .../@ember/-internals/glimmer/lib/resolver.ts | 6 ++- .../-internals/glimmer/lib/setup-registry.ts | 1 + .../glimmer/lib/template-compiler.ts | 10 ++++- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts index 4829f477b09..b4154056b6a 100644 --- a/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts +++ b/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts @@ -1,6 +1,6 @@ import { Factory } from '@ember/-internals/owner'; import { Dict, Opaque, Simple } from '@glimmer/interfaces'; -import { Tag } from '@glimmer/reference'; +import { CONSTANT_TAG, Tag } from '@glimmer/reference'; import { Arguments, CapturedArguments, ModifierManager } from '@glimmer/runtime'; export interface CustomModifierDefinitionState { @@ -18,17 +18,23 @@ export function capabilities(_managerAPI: string, _optionalFeatures?: {}): Capab export class CustomModifierDefinition { public state: CustomModifierDefinitionState; - public manager = CUSTOM_MODIFIER_MANAGER; + public manager: ModifierManager>; + constructor( public name: string, public ModifierClass: Factory, - public delegate: ModifierManagerDelegate + public delegate: ModifierManagerDelegate, + isInteractive: boolean ) { this.state = { ModifierClass, name, delegate, }; + + this.manager = isInteractive + ? CUSTOM_INTERACTIVE_MODIFIER_MANAGER + : CUSTOM_NON_INTERACTIVE_MODIFIER_MANAGER; } } @@ -67,12 +73,15 @@ export interface ModifierManagerDelegate { implements a set of hooks that determine modifier behavior. To create a custom modifier manager, instantiate a new CustomModifierManager class and pass the delegate as the first argument: + ```js let manager = new CustomModifierManager({ // ...delegate implementation... }); ``` + ## Delegate Hooks + Throughout the lifecycle of a modifier, the modifier manager will invoke delegate hooks that are responsible for surfacing those lifecycle changes to the end developer. @@ -81,7 +90,7 @@ export interface ModifierManagerDelegate { * `updateModifier()` - invoked when the arguments passed to a modifier change * `destroyModifier()` - invoked when the modifier is about to be destroyed */ -class CustomModifierManager +class InteractiveCustomModifierManager implements ModifierManager< CustomModifierState, @@ -119,4 +128,24 @@ class CustomModifierManager } } -const CUSTOM_MODIFIER_MANAGER = new CustomModifierManager(); +class NonInteractiveCustomModifierManager + implements ModifierManager> { + create() { + return null; + } + + getTag(): Tag { + return CONSTANT_TAG; + } + + install() {} + + update() {} + + getDestructor() { + return null; + } +} + +const CUSTOM_INTERACTIVE_MODIFIER_MANAGER = new InteractiveCustomModifierManager(); +const CUSTOM_NON_INTERACTIVE_MODIFIER_MANAGER = new NonInteractiveCustomModifierManager(); diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 93a53d67ff2..ba9be164df3 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -109,6 +109,7 @@ if (EMBER_GLIMMER_ON_MODIFIER) { BUILTIN_MODIFIERS.on = { manager: new OnModifierManager(), state: null }; } export default class RuntimeResolver implements IRuntimeResolver { + public isInteractive: boolean; public compiler: LazyCompiler; private handles: any[] = [ @@ -130,10 +131,11 @@ export default class RuntimeResolver implements IRuntimeResolver(new CompileTimeLookup(this), this, macros); + this.isInteractive = isInteractive; } /*** IRuntimeResolver ***/ @@ -306,7 +308,7 @@ export default class RuntimeResolver implements IRuntimeResolver>(modifier.class); let manager = managerFactory!(owner); - return new CustomModifierDefinition(name, modifier, manager); + return new CustomModifierDefinition(name, modifier, manager, this.isInteractive); } } diff --git a/packages/@ember/-internals/glimmer/lib/setup-registry.ts b/packages/@ember/-internals/glimmer/lib/setup-registry.ts index 46d1303c149..6d9154652e1 100644 --- a/packages/@ember/-internals/glimmer/lib/setup-registry.ts +++ b/packages/@ember/-internals/glimmer/lib/setup-registry.ts @@ -92,6 +92,7 @@ export function setupEngineRegistry(registry: Registry) { registry.register('service:-glimmer-environment', Environment); registry.register(P`template-compiler:main`, TemplateCompiler); + registry.injection(P`template-compiler:main`, 'environment', '-environment:main'); registry.injection('template', 'compiler', P`template-compiler:main`); diff --git a/packages/@ember/-internals/glimmer/lib/template-compiler.ts b/packages/@ember/-internals/glimmer/lib/template-compiler.ts index 1e67214d7b5..cedd4d93acd 100644 --- a/packages/@ember/-internals/glimmer/lib/template-compiler.ts +++ b/packages/@ember/-internals/glimmer/lib/template-compiler.ts @@ -1,9 +1,15 @@ import { Compiler } from '@glimmer/interfaces'; import RuntimeResolver from './resolver'; +export interface ICompilerOptions { + environment: { + isInteractive: boolean; + }; +} + // factory for DI export default { - create(): Compiler { - return new RuntimeResolver().compiler; + create({ environment }: ICompilerOptions): Compiler { + return new RuntimeResolver(environment.isInteractive).compiler; }, }; From a82907f6793dedebb41452645433dd2844dfc5ad Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 3 Jun 2019 16:51:35 -0400 Subject: [PATCH 4/4] [BUGFIX beta] Ensure `{{on}}` does not run in FastBoot / SSR contexts. Since `{{on}}` is a "special" built-in modifier manager, the generic fixes for custom modifier managers do not fix `on`. This applies the same general fix, and ensures that `{{on}}` is a no-op in FastBoot mode. --- .../-internals/glimmer/lib/modifiers/on.ts | 35 +++++++++++++++---- .../@ember/-internals/glimmer/lib/resolver.ts | 17 ++++----- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/modifiers/on.ts b/packages/@ember/-internals/glimmer/lib/modifiers/on.ts index 54cbc4f2f1e..f4ca6018d6e 100644 --- a/packages/@ember/-internals/glimmer/lib/modifiers/on.ts +++ b/packages/@ember/-internals/glimmer/lib/modifiers/on.ts @@ -1,7 +1,7 @@ import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { Opaque, Simple } from '@glimmer/interfaces'; -import { Tag } from '@glimmer/reference'; +import { CONSTANT_TAG, Tag } from '@glimmer/reference'; import { Arguments, CapturedArguments, ModifierManager } from '@glimmer/runtime'; import { Destroyable } from '@glimmer/util'; import buildUntouchableThis from '../utils/untouchable-this'; @@ -210,24 +210,41 @@ function addEventListener( } } -export default class OnModifierManager implements ModifierManager { +export default class OnModifierManager implements ModifierManager { public SUPPORTS_EVENT_OPTIONS: boolean = SUPPORTS_EVENT_OPTIONS; + public isInteractive: boolean; + + constructor(isInteractive: boolean) { + this.isInteractive = isInteractive; + } get counters() { return { adds, removes }; } create(element: Simple.Element | Element, _state: Opaque, args: Arguments) { + if (!this.isInteractive) { + return null; + } + const capturedArgs = args.capture(); return new OnModifierState(element, capturedArgs); } - getTag({ tag }: OnModifierState): Tag { - return tag; + getTag(state: OnModifierState | null): Tag { + if (state === null) { + return CONSTANT_TAG; + } + + return state.tag; } - install(state: OnModifierState) { + install(state: OnModifierState | null) { + if (state === null) { + return; + } + state.updateFromArgs(); let { element, eventName, callback, options } = state; @@ -237,7 +254,11 @@ export default class OnModifierManager implements ModifierManager { public isInteractive: boolean; public compiler: LazyCompiler; @@ -119,7 +112,7 @@ export default class RuntimeResolver implements IRuntimeResolver> = new Map(); @@ -136,6 +129,14 @@ export default class RuntimeResolver implements IRuntimeResolver(new CompileTimeLookup(this), this, macros); this.isInteractive = isInteractive; + + this.builtInModifiers = { + action: { manager: new ActionModifierManager(), state: null }, + }; + + if (EMBER_GLIMMER_ON_MODIFIER) { + this.builtInModifiers.on = { manager: new OnModifierManager(isInteractive), state: null }; + } } /*** IRuntimeResolver ***/