Skip to content

Commit

Permalink
[FEATURE strict-mode] Update VM for Strict Mode
Browse files Browse the repository at this point in the history
Updates the Glimmer VM to the latest version, which includes strict
mode. Strict mode is currently guarded behind a canary flag, but for the
most part only involves changes to the VM. The biggest changes are to
the template compiler's `compile` function, which has to change in order
to still be functional as scope values must be provided in some way, and
the resolver, which needs to provide keyword built-ins now in strict
templates.

Release notes for the VM: https://github.com/glimmerjs/glimmer-vm/releases/tag/v0.69.0
  • Loading branch information
Chris Garrett committed Dec 10, 2020
1 parent 23ee5d1 commit 8180b67
Show file tree
Hide file tree
Showing 19 changed files with 468 additions and 193 deletions.
24 changes: 12 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@
},
"devDependencies": {
"@babel/preset-env": "^7.9.5",
"@glimmer/compiler": "0.68.1",
"@glimmer/compiler": "0.69.1",
"@glimmer/env": "^0.1.7",
"@glimmer/global-context": "0.68.1",
"@glimmer/interfaces": "0.68.1",
"@glimmer/manager": "0.68.1",
"@glimmer/destroyable": "0.68.1",
"@glimmer/owner": "0.68.1",
"@glimmer/node": "0.68.1",
"@glimmer/opcode-compiler": "0.68.1",
"@glimmer/program": "0.68.1",
"@glimmer/reference": "0.68.1",
"@glimmer/runtime": "0.68.1",
"@glimmer/validator": "0.68.1",
"@glimmer/global-context": "0.69.1",
"@glimmer/interfaces": "0.69.1",
"@glimmer/manager": "0.69.1",
"@glimmer/destroyable": "0.69.1",
"@glimmer/owner": "0.69.1",
"@glimmer/node": "0.69.1",
"@glimmer/opcode-compiler": "0.69.1",
"@glimmer/program": "0.69.1",
"@glimmer/reference": "0.69.1",
"@glimmer/runtime": "0.69.1",
"@glimmer/validator": "0.69.1",
"@simple-dom/document": "^1.4.0",
"@types/qunit": "^2.9.1",
"@types/rsvp": "^4.0.3",
Expand Down
40 changes: 28 additions & 12 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,15 @@ if (PARTIALS) {
};
}

const BUILTINS_HELPERS = {
if: inlineIf,
const BUILTIN_KEYWORD_HELPERS = {
action,
array,
concat,
fn,
get,
hash,
if: inlineIf,
log,
mut,
'query-params': queryParams,
readonly,
unbound,
unless: inlineUnless,
'query-params': queryParams,
'-hash': hash,
'-each-in': eachIn,
'-normalize-class': normalizeClassHelper,
Expand All @@ -202,8 +197,21 @@ const BUILTINS_HELPERS = {
'-in-el-null': inElementNullCheckHelper,
};

const BUILTINS_MODIFIERS = {
const BUILTIN_HELPERS = {
...BUILTIN_KEYWORD_HELPERS,
array,
concat,
fn,
get,
hash,
};

const BUILTIN_KEYWORD_MODIFIERS = {
action: actionModifier,
};

const BUILTIN_MODIFIERS = {
...BUILTIN_KEYWORD_MODIFIERS,
on: onModifier,
};

Expand All @@ -226,10 +234,10 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
lookupHelper(name: string, owner: Owner): Option<HelperDefinitionState> {
assert(
`You attempted to overwrite the built-in helper "${name}" which is not allowed. Please rename the helper.`,
!(BUILTINS_HELPERS[name] && owner.hasRegistration(`helper:${name}`))
!(BUILTIN_HELPERS[name] && owner.hasRegistration(`helper:${name}`))
);

const helper = BUILTINS_HELPERS[name];
const helper = BUILTIN_HELPERS[name];
if (helper !== undefined) {
return helper;
}
Expand Down Expand Up @@ -271,8 +279,12 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
return definition;
}

lookupBuiltInHelper(name: string): HelperDefinitionState | null {
return BUILTIN_KEYWORD_HELPERS[name] ?? null;
}

lookupModifier(name: string, owner: Owner): Option<ModifierDefinitionState> {
let builtin = BUILTINS_MODIFIERS[name];
let builtin = BUILTIN_MODIFIERS[name];

if (builtin !== undefined) {
return builtin;
Expand All @@ -287,6 +299,10 @@ export default class ResolverImpl implements RuntimeResolver<Owner>, CompileTime
return modifier.class || null;
}

lookupBuiltInModifier(name: string): ModifierDefinitionState | null {
return BUILTIN_KEYWORD_MODIFIERS[name] ?? null;
}

lookupComponent(name: string, owner: Owner): ResolvedComponentDefinition | null {
let pair = lookupComponentPair(owner, name);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { componentCapabilities, setComponentTemplate } from '@glimmer/manager';
import { templateOnlyComponent } from '@glimmer/runtime';
import { expect } from '@glimmer/util';
import { SimpleElement, SimpleNode } from '@simple-dom/interface';
import { compile } from 'ember-template-compiler';
import { compile, EmberPrecompileOptions } from 'ember-template-compiler';
import { runTask } from 'internal-test-helpers/lib/run';

interface CapturedBounds {
Expand All @@ -27,6 +27,10 @@ interface CapturedBounds {
lastNode: SimpleNode;
}

function compileTemplate(templateSource: string, options: Partial<EmberPrecompileOptions>) {
return compile(templateSource, options) as any;
}

type Expected<T> = T | ((actual: T) => boolean);

function isExpectedFunc<T>(expected: Expected<T>): expected is (actual: T) => boolean {
Expand Down Expand Up @@ -301,7 +305,7 @@ if (ENV._DEBUG_RENDER_TREE) {
super.init(...arguments);
this.register(
'template:application',
compile(
compileTemplate(
strip`
{{#if @model}}
<InspectModel @model={{@model}} />
Expand All @@ -314,7 +318,7 @@ if (ENV._DEBUG_RENDER_TREE) {
);
this.register(
'template:components/inspect-model',
compile('{{@model}}', {
compileTemplate('{{@model}}', {
moduleName: 'foo/components/inspect-model.hbs',
})
);
Expand All @@ -337,7 +341,7 @@ if (ENV._DEBUG_RENDER_TREE) {
super.init(...arguments);
this.register(
'template:application',
compile(
compileTemplate(
strip`
{{#if @model}}
<InspectModel @model={{@model}} />
Expand All @@ -350,7 +354,7 @@ if (ENV._DEBUG_RENDER_TREE) {
);
this.register(
'template:components/inspect-model',
compile('{{@model}}', {
compileTemplate('{{@model}}', {
moduleName: 'bar/components/inspect-model.hbs',
})
);
Expand Down Expand Up @@ -682,7 +686,7 @@ if (ENV._DEBUG_RENDER_TREE) {
super.init(...arguments);
this.register(
'template:application',
compile(
compileTemplate(
strip`
{{outlet}}
Expand All @@ -697,13 +701,13 @@ if (ENV._DEBUG_RENDER_TREE) {
);
this.register(
'template:index',
compile('Foo', {
compileTemplate('Foo', {
moduleName: 'foo/templates/index.hbs',
})
);
this.register(
'template:components/hello',
compile('<span>Hello {{@message}}</span>', {
compileTemplate('<span>Hello {{@message}}</span>', {
moduleName: 'foo/components/hello.hbs',
})
);
Expand Down Expand Up @@ -1027,7 +1031,7 @@ if (ENV._DEBUG_RENDER_TREE) {

this.addComponent('hello-world', {
ComponentClass: setComponentTemplate(
compile('{{@name}}', { moduleName: 'my-app/components/hello-world.hbs' }),
compileTemplate('{{@name}}', { moduleName: 'my-app/components/hello-world.hbs' }),
templateOnlyComponent()
),
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import {
moduleFor,
RenderingTestCase,
defineComponent,
defineSimpleHelper,
defineSimpleModifier,
} from 'internal-test-helpers';
import { EMBER_STRICT_MODE } from '@ember/canary-features';

if (EMBER_STRICT_MODE) {
moduleFor(
'Strict Mode',
class extends RenderingTestCase {
'@test Can use a component in scope'() {
let Foo = defineComponent({}, 'Hello, world!');
let Bar = defineComponent({ Foo }, '<Foo/>');

this.registerComponent('bar', { ComponentClass: Bar });

this.render('<Bar/>');
this.assertHTML('Hello, world!');
this.assertStableRerender();
}

'@test Can use a custom helper in scope (in append position)'() {
let foo = defineSimpleHelper(() => 'Hello, world!');
let Bar = defineComponent({ foo }, '{{foo}}');

this.registerComponent('bar', { ComponentClass: Bar });

this.render('<Bar/>');
this.assertHTML('Hello, world!');
this.assertStableRerender();
}

'@test Can use a custom modifier in scope'() {
let foo = defineSimpleModifier((element) => (element.innerHTML = 'Hello, world!'));
let Bar = defineComponent({ foo }, '<div {{foo}}></div>');

this.registerComponent('bar', { ComponentClass: Bar });

this.render('<Bar/>');
this.assertHTML('<div>Hello, world!</div>');
this.assertStableRerender();
}

'@test Can shadow keywords'() {
let ifComponent = defineComponent({}, 'Hello, world!');
let Bar = defineComponent({ if: ifComponent }, '{{#if}}{{/if}}');

this.registerComponent('bar', { ComponentClass: Bar });

this.render('<Bar/>');
this.assertHTML('Hello, world!');
this.assertStableRerender();
}

'@test Can use constant values in ambiguous helper/component position'() {
let value = 'Hello, world!';

let Foo = defineComponent({ value }, '{{value}}');

this.registerComponent('foo', { ComponentClass: Foo });

this.render('<Foo/>');
this.assertHTML('Hello, world!');
this.assertStableRerender();
}

'@test Can use inline if and unless in strict mode templates'() {
let Foo = defineComponent({}, '{{if true "foo" "bar"}}{{unless true "foo" "bar"}}');

this.registerComponent('foo', { ComponentClass: Foo });

this.render('<Foo/>');
this.assertHTML('foobar');
this.assertStableRerender();
}
}
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ if (EMBER_GLIMMER_INVOKE_HELPER) {

assert.throws(() => {
invokeHelper({}, class {});
}, /Attempted to load a helper, but there wasn't a manager associated with the definition. The definition was:/);
}, /Attempted to load a helper, but there wasn't a helper manager associated with the definition. The definition was:/);
}
}
);
Expand Down
3 changes: 2 additions & 1 deletion packages/@ember/-internals/glimmer/tests/utils/helpers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { compile, precompile } from 'ember-template-compiler';
export { precompile } from 'ember-template-compiler';
export { compile } from 'internal-test-helpers';

export {
INVOKE,
Expand Down
2 changes: 2 additions & 0 deletions packages/@ember/canary-features/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const DEFAULT_FEATURES = {
EMBER_GLIMMER_HELPER_MANAGER: true,
EMBER_GLIMMER_INVOKE_HELPER: true,
EMBER_MODERNIZED_BUILT_IN_COMPONENTS: null,
EMBER_STRICT_MODE: null,
};

/**
Expand Down Expand Up @@ -75,3 +76,4 @@ export const EMBER_GLIMMER_INVOKE_HELPER = featureValue(FEATURES.EMBER_GLIMMER_I
export const EMBER_MODERNIZED_BUILT_IN_COMPONENTS = featureValue(
FEATURES.EMBER_MODERNIZED_BUILT_IN_COMPONENTS
);
export const EMBER_STRICT_MODE = featureValue(FEATURES.EMBER_STRICT_MODE);
1 change: 1 addition & 0 deletions packages/ember-template-compiler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export {
unregisterPlugin,
} from './lib/system/compile-options';
export { default as defaultPlugins } from './lib/plugins/index';
export { EmberPrecompileOptions } from './lib/types';

// used to bootstrap templates
import './lib/system/bootstrap';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ export default function assertLocalVariableShadowingHelperInvocation(
let { moduleName } = env.meta;
let { hasLocal, node } = trackLocals();

if (env.strictMode) {
return {
name: 'assert-local-variable-shadowing-helper-invocation',
visitor: {},
};
}

return {
name: 'assert-local-variable-shadowing-helper-invocation',

Expand Down
3 changes: 2 additions & 1 deletion packages/ember-template-compiler/lib/system/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
@module ember
*/

import { TemplateFactory } from '@glimmer/interfaces';
import compile from './compile';

export interface BootstrapOptions {
context?: Document | HTMLElement;
hasTemplate(templateName: string): boolean;
setTemplate(templateName: string, template: string): void;
setTemplate(templateName: string, template: TemplateFactory): void;
}

/**
Expand Down
16 changes: 12 additions & 4 deletions packages/ember-template-compiler/lib/system/compile-options.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { EMBER_STRICT_MODE } from '@ember/canary-features';
import { assign } from '@ember/polyfills';
import { PrecompileOptions } from '@glimmer/compiler';
import { AST, ASTPlugin, ASTPluginEnvironment, Syntax } from '@glimmer/syntax';
import PLUGINS from '../plugins/index';
import { CompileOptions, EmberPrecompileOptions, PluginFunc } from '../types';
import { EmberPrecompileOptions, PluginFunc } from '../types';
import COMPONENT_NAME_SIMPLE_DASHERIZE_CACHE from './dasherize-component-name';

let USER_PLUGINS: PluginFunc[] = [];

export default function compileOptions(
_options: Partial<CompileOptions> = {}
): EmberPrecompileOptions {
_options: Partial<EmberPrecompileOptions> = {}
): PrecompileOptions {
let options: EmberPrecompileOptions = assign(
{ meta: {}, isProduction: false, plugins: { ast: [] } },
_options,
Expand All @@ -19,6 +21,11 @@ export default function compileOptions(
}
);

if (!EMBER_STRICT_MODE) {
options.strictMode = false;
options.locals = undefined;
}

// move `moduleName` into `meta` property
if (options.moduleName) {
let meta = options.meta;
Expand All @@ -36,7 +43,8 @@ export default function compileOptions(
options.plugins.ast = providedPlugins.concat(pluginsToAdd);
}

return options;
// TODO: Fix the types here so that this conversion isn't necessary
return (options as unknown) as PrecompileOptions;
}

interface LegacyPlugin {
Expand Down
Loading

0 comments on commit 8180b67

Please sign in to comment.