Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX lts + beta] Ensure modifiers do not run in FastBoot modes. #18071

Merged
merged 4 commits into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions packages/@ember/-internals/glimmer/lib/modifiers/custom.ts
Original file line number Diff line number Diff line change
@@ -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<ModifierInstance> {
Expand All @@ -18,17 +18,23 @@ export function capabilities(_managerAPI: string, _optionalFeatures?: {}): Capab

export class CustomModifierDefinition<ModifierInstance> {
public state: CustomModifierDefinitionState<ModifierInstance>;
public manager = CUSTOM_MODIFIER_MANAGER;
public manager: ModifierManager<unknown | null, CustomModifierDefinitionState<ModifierInstance>>;

constructor(
public name: string,
public ModifierClass: Factory<ModifierInstance>,
public delegate: ModifierManagerDelegate<ModifierInstance>
public delegate: ModifierManagerDelegate<ModifierInstance>,
isInteractive: boolean
) {
this.state = {
ModifierClass,
name,
delegate,
};

this.manager = isInteractive
? CUSTOM_INTERACTIVE_MODIFIER_MANAGER
: CUSTOM_NON_INTERACTIVE_MODIFIER_MANAGER;
}
}

Expand Down Expand Up @@ -67,12 +73,15 @@ export interface ModifierManagerDelegate<ModifierInstance> {
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.
Expand All @@ -81,7 +90,7 @@ export interface ModifierManagerDelegate<ModifierInstance> {
* `updateModifier()` - invoked when the arguments passed to a modifier change
* `destroyModifier()` - invoked when the modifier is about to be destroyed
*/
class CustomModifierManager<ModifierInstance>
class InteractiveCustomModifierManager<ModifierInstance>
implements
ModifierManager<
CustomModifierState<ModifierInstance>,
Expand Down Expand Up @@ -119,4 +128,24 @@ class CustomModifierManager<ModifierInstance>
}
}

const CUSTOM_MODIFIER_MANAGER = new CustomModifierManager();
class NonInteractiveCustomModifierManager<ModifierInstance>
implements ModifierManager<null, CustomModifierDefinitionState<ModifierInstance>> {
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();
35 changes: 28 additions & 7 deletions packages/@ember/-internals/glimmer/lib/modifiers/on.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -210,24 +210,41 @@ function addEventListener(
}
}

export default class OnModifierManager implements ModifierManager<OnModifierState, Opaque> {
export default class OnModifierManager implements ModifierManager<OnModifierState | null, Opaque> {
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>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;
Expand All @@ -237,7 +254,11 @@ export default class OnModifierManager implements ModifierManager<OnModifierStat
state.shouldUpdate = false;
}

update(state: OnModifierState) {
update(state: OnModifierState | null) {
if (state === null) {
return;
}

// stash prior state for el.removeEventListener
let { element, eventName, callback, options } = state;

Expand All @@ -256,7 +277,7 @@ export default class OnModifierManager implements ModifierManager<OnModifierStat
state.shouldUpdate = false;
}

getDestructor(state: Destroyable) {
getDestructor(state: Destroyable | null) {
return state;
}
}
23 changes: 13 additions & 10 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,9 @@ if (EMBER_GLIMMER_FN_HELPER) {
interface IBuiltInModifiers {
[name: string]: ModifierDefinition | undefined;
}
const BUILTIN_MODIFIERS: IBuiltInModifiers = {
action: { manager: new ActionModifierManager(), state: null },
on: undefined,
};

if (EMBER_GLIMMER_ON_MODIFIER) {
BUILTIN_MODIFIERS.on = { manager: new OnModifierManager(), state: null };
}
export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMeta> {
public isInteractive: boolean;
public compiler: LazyCompiler<OwnedTemplateMeta>;

private handles: any[] = [
Expand All @@ -118,7 +112,7 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe

private builtInHelpers: IBuiltInHelpers = BUILTINS_HELPERS;

private builtInModifiers: IBuiltInModifiers = BUILTIN_MODIFIERS;
private builtInModifiers: IBuiltInModifiers;

// supports directly imported late bound layouts on component.prototype.layout
private templateCache: Map<Owner, Map<TemplateFactory, OwnedTemplate>> = new Map();
Expand All @@ -130,10 +124,19 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
public componentDefinitionCount = 0;
public helperDefinitionCount = 0;

constructor() {
constructor(isInteractive: boolean) {
let macros = new Macros();
populateMacros(macros);
this.compiler = new LazyCompiler<OwnedTemplateMeta>(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 ***/
Expand Down Expand Up @@ -306,7 +309,7 @@ export default class RuntimeResolver implements IRuntimeResolver<OwnedTemplateMe
let managerFactory = getModifierManager<ModifierManagerDelegate<Opaque>>(modifier.class);
let manager = managerFactory!(owner);

return new CustomModifierDefinition(name, modifier, manager);
return new CustomModifierDefinition(name, modifier, manager, this.isInteractive);
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/@ember/-internals/glimmer/lib/setup-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);

Expand Down
10 changes: 8 additions & 2 deletions packages/@ember/-internals/glimmer/lib/template-compiler.ts
Original file line number Diff line number Diff line change
@@ -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;
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -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('<h1 {{foo-bar baz}}>hello world</h1>');
runTask(() => this.context.set('baz', 'Hello'));

this.assertHTML('<h1>hello world</h1>');
}
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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: `<button {{on 'click' this.fire}}>Fire!</button>`,
});

this.render('{{#if this.showButton}}<FooBar2 />{{/if}}', {
showButton: true,
});
this.assertHTML('<button>Fire!</button>');
this.assertCounts({ adds: 0, removes: 0 });

this.$('button').click();

runTask(() => this.context.set('showButton', false));

this.assertCounts({ adds: 0, removes: 0 });
}
}
);
}