From 998243e35279ea8042179af07f78bdf9e12b1e4c Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Wed, 9 Dec 2020 21:30:55 -0800 Subject: [PATCH] [FEATURE strict-mode] Update VM for Strict Mode 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 --- package.json | 24 +- packages/@ember/-internals/glimmer/index.ts | 9 +- .../glimmer/lib/component-managers/curly.ts | 15 +- .../lib/component-managers/internal.ts | 18 +- .../glimmer/lib/components/input.ts | 16 +- .../glimmer/lib/components/internal.ts | 6 - .../@ember/-internals/glimmer/lib/resolver.ts | 51 ++-- .../-internals/glimmer/lib/setup-registry.ts | 10 +- .../application/debug-render-tree-test.ts | 22 +- .../components/strict-mode-test.js | 217 ++++++++++++++ .../integration/helpers/invoke-helper-test.js | 2 +- .../-internals/glimmer/tests/utils/helpers.js | 3 +- packages/@ember/canary-features/index.ts | 2 + packages/ember-template-compiler/index.ts | 1 + ...al-variable-shadowing-helper-invocation.ts | 7 + .../lib/system/bootstrap.ts | 3 +- .../lib/system/compile-options.ts | 16 +- .../lib/system/compile.ts | 9 +- .../lib/system/precompile.ts | 13 +- .../ember-template-compiler/lib/types.d.ts | 14 +- .../tests/utils/transform-test-case.ts | 2 +- packages/ember/index.js | 15 + packages/internal-test-helpers/index.js | 6 + packages/internal-test-helpers/lib/compile.ts | 39 +++ .../lib/define-template-values.ts | 113 ++++++++ yarn.lock | 264 +++++++++--------- 26 files changed, 676 insertions(+), 221 deletions(-) create mode 100644 packages/@ember/-internals/glimmer/tests/integration/components/strict-mode-test.js create mode 100644 packages/internal-test-helpers/lib/compile.ts create mode 100644 packages/internal-test-helpers/lib/define-template-values.ts diff --git a/package.json b/package.json index 5e133f5d93f..d166b52da72 100644 --- a/package.json +++ b/package.json @@ -74,19 +74,19 @@ }, "devDependencies": { "@babel/preset-env": "^7.9.5", - "@glimmer/compiler": "0.68.1", + "@glimmer/compiler": "0.69.3", "@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.3", + "@glimmer/interfaces": "0.69.3", + "@glimmer/manager": "0.69.3", + "@glimmer/destroyable": "0.69.3", + "@glimmer/owner": "0.69.3", + "@glimmer/node": "0.69.3", + "@glimmer/opcode-compiler": "0.69.3", + "@glimmer/program": "0.69.3", + "@glimmer/reference": "0.69.3", + "@glimmer/runtime": "0.69.3", + "@glimmer/validator": "0.69.3", "@simple-dom/document": "^1.4.0", "@types/qunit": "^2.9.1", "@types/rsvp": "^4.0.3", diff --git a/packages/@ember/-internals/glimmer/index.ts b/packages/@ember/-internals/glimmer/index.ts index 7729ebecc57..72e3fc4362d 100644 --- a/packages/@ember/-internals/glimmer/index.ts +++ b/packages/@ember/-internals/glimmer/index.ts @@ -367,6 +367,7 @@ export { default as Checkbox } from './lib/components/checkbox'; export { default as TextField } from './lib/components/text-field'; export { default as TextArea } from './lib/components/textarea'; export { default as LinkComponent } from './lib/components/link-to'; +export { InputComponent as Input } from './lib/components/input'; export { default as Component } from './lib/component'; export { default as Helper, helper } from './lib/helper'; export { SafeString, escapeExpression, htmlSafe, isHTMLSafe } from './lib/utils/string'; @@ -387,12 +388,18 @@ export { export { setupEngineRegistry, setupApplicationRegistry } from './lib/setup-registry'; export { DOMChanges, NodeDOMTreeConstruction, DOMTreeConstruction } from './lib/dom'; +export { default as array } from './lib/helpers/array'; +export { default as hash } from './lib/helpers/hash'; +export { default as concat } from './lib/helpers/concat'; +export { default as get } from './lib/helpers/get'; +export { default as fn } from './lib/helpers/fn'; +export { default as on } from './lib/modifiers/on'; + // needed for test // TODO just test these through public API // a lot of these are testing how a problem was solved // rather than the problem was solved export { INVOKE } from './lib/helpers/action'; -export { default as on } from './lib/modifiers/on'; export { default as OutletView } from './lib/views/outlet'; export { OutletState } from './lib/utils/outlet'; export { setComponentManager } from './lib/utils/managers'; diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 05ec993483d..a78ed8c84c1 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -124,7 +124,10 @@ type ComponentFactory = Factory< positionalParams: string | string[] | undefined | null; name: string; } ->; +> & { + name: string; + positionalParams: string | string[] | undefined | null; +}; export default class CurlyComponentManager implements @@ -188,7 +191,7 @@ export default class CurlyComponentManager return prepared; } - const { positionalParams } = ComponentClass.class!; + const { positionalParams } = ComponentClass.class ?? ComponentClass; // early exits if ( @@ -355,7 +358,9 @@ export default class CurlyComponentManager } getDebugName(definition: ComponentFactory): string { - return definition.fullName || definition.normalizedName || definition.class!.name; + return ( + definition.fullName || definition.normalizedName || definition.class?.name || definition.name + ); } getSelf({ rootRef }: ComponentStateBucket): Reference { @@ -551,3 +556,7 @@ export const CURLY_CAPABILITIES: InternalComponentCapabilities = { }; export const CURLY_COMPONENT_MANAGER = new CurlyComponentManager(); + +export function isCurlyManager(manager: object): boolean { + return manager === CURLY_COMPONENT_MANAGER; +} diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/internal.ts b/packages/@ember/-internals/glimmer/lib/component-managers/internal.ts index aa30f4f9e5c..c29ea9f99ef 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/internal.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/internal.ts @@ -40,11 +40,18 @@ export default class InternalManager implements InternalComponentManager, WithCreateInstance { - static for(name: string): (owner: Owner) => InternalManager { - return (owner: Owner) => new InternalManager(owner, name); + static for( + definition: EmberInternalComponentConstructor, + name: string + ): (owner: Owner) => InternalManager { + return (owner: Owner) => new InternalManager(owner, definition, name); } - constructor(private owner: Owner, private name: string) {} + constructor( + private owner: Owner, + private ComponentClass: EmberInternalComponentConstructor, + private name: string + ) {} getCapabilities(): InternalComponentCapabilities { return CAPABILITIES; @@ -52,7 +59,7 @@ export default class InternalManager create( env: Environment, - ComponentClass: EmberInternalComponentConstructor, + _definition: unknown, args: VMArguments, _dynamicScope: DynamicScope, caller: Reference @@ -64,7 +71,8 @@ export default class InternalManager args.positional.length === 0 ); - let instance = new ComponentClass(this.owner, args.named.capture(), valueForRef(caller)); + let { ComponentClass, owner } = this; + let instance = new ComponentClass(owner, args.named.capture(), valueForRef(caller)); let state = { env, instance }; diff --git a/packages/@ember/-internals/glimmer/lib/components/input.ts b/packages/@ember/-internals/glimmer/lib/components/input.ts index e56943537dc..eb2c7c3c4f3 100644 --- a/packages/@ember/-internals/glimmer/lib/components/input.ts +++ b/packages/@ember/-internals/glimmer/lib/components/input.ts @@ -1,10 +1,11 @@ /** @module @ember/component */ -import { setInternalComponentManager } from '@glimmer/manager'; +import { assert } from '@ember/debug'; +import { setComponentTemplate, setInternalComponentManager } from '@glimmer/manager'; import InternalManager from '../component-managers/internal'; +import InputTemplate from '../templates/input'; import InternalComponent from './internal'; - /** See [Ember.Templates.components.Input](/ember/release/classes/Ember.Templates.components/methods/Input?anchor=Input). @@ -117,6 +118,15 @@ export default class Input extends InternalComponent { } } -setInternalComponentManager(InternalManager.for('input'), Input); +// Use an opaque handle so implementation details are +export const InputComponent = { + // Factory interface + create(): never { + throw assert('Use constructor instead of create'); + }, +}; + +setInternalComponentManager(InternalManager.for(Input, 'input'), InputComponent); +setComponentTemplate(InputTemplate, InputComponent); Input.toString = () => '@ember/component/input'; diff --git a/packages/@ember/-internals/glimmer/lib/components/internal.ts b/packages/@ember/-internals/glimmer/lib/components/internal.ts index b6b41f4d911..406985a14ca 100644 --- a/packages/@ember/-internals/glimmer/lib/components/internal.ts +++ b/packages/@ember/-internals/glimmer/lib/components/internal.ts @@ -1,14 +1,8 @@ import { Owner, setOwner } from '@ember/-internals/owner'; import { guidFor } from '@ember/-internals/utils'; -import { assert } from '@ember/debug'; import { Reference, valueForRef } from '@glimmer/reference'; export default class InternalComponent { - // Factory interface - static create(): never { - throw assert('Use constructor instead of create'); - } - static get class(): typeof InternalComponent { return this; } diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 1f7eec6f6c0..529601e1ddd 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -29,7 +29,7 @@ import { templateOnlyComponent, } from '@glimmer/runtime'; import { _WeakSet } from '@glimmer/util'; -import { CURLY_COMPONENT_MANAGER } from './component-managers/curly'; +import { isCurlyManager } from './component-managers/curly'; import { CLASSIC_HELPER_MANAGER_FACTORY, HelperFactory, @@ -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, @@ -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, }; @@ -226,10 +234,10 @@ export default class ResolverImpl implements RuntimeResolver, CompileTime lookupHelper(name: string, owner: Owner): Option { 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; } @@ -271,8 +279,12 @@ export default class ResolverImpl implements RuntimeResolver, CompileTime return definition; } + lookupBuiltInHelper(name: string): HelperDefinitionState | null { + return BUILTIN_KEYWORD_HELPERS[name] ?? null; + } + lookupModifier(name: string, owner: Owner): Option { - let builtin = BUILTINS_MODIFIERS[name]; + let builtin = BUILTIN_MODIFIERS[name]; if (builtin !== undefined) { return builtin; @@ -287,6 +299,10 @@ export default class ResolverImpl implements RuntimeResolver, 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); @@ -328,9 +344,12 @@ export default class ResolverImpl implements RuntimeResolver, CompileTime template, }; } else { + let factory = owner.factoryFor(P`component:-default`)!; + let manager = getInternalComponentManager(owner, factory.class as object); + definition = { - state: owner.factoryFor(P`component:-default`)!, - manager: CURLY_COMPONENT_MANAGER, + state: factory, + manager, template, }; } @@ -342,7 +361,7 @@ export default class ResolverImpl implements RuntimeResolver, CompileTime let manager = getInternalComponentManager(owner, ComponentClass); definition = { - state: manager === CURLY_COMPONENT_MANAGER ? factory : ComponentClass, + state: isCurlyManager(manager) ? factory : ComponentClass, manager, template, }; diff --git a/packages/@ember/-internals/glimmer/lib/setup-registry.ts b/packages/@ember/-internals/glimmer/lib/setup-registry.ts index 5d4b384e30e..58bf4ae8f27 100644 --- a/packages/@ember/-internals/glimmer/lib/setup-registry.ts +++ b/packages/@ember/-internals/glimmer/lib/setup-registry.ts @@ -2,19 +2,18 @@ import { privatize as P, Registry } from '@ember/-internals/container'; import { ENV } from '@ember/-internals/environment'; import Component from './component'; import Checkbox from './components/checkbox'; -import Input from './components/input'; +import { InputComponent } from './components/input'; import LinkToComponent from './components/link-to'; import TextField from './components/text-field'; import TextArea from './components/textarea'; import { clientBuilder, rehydrationBuilder, serializeBuilder } from './dom'; import loc from './helpers/loc'; import { InertRenderer, InteractiveRenderer } from './renderer'; -import InputTemplate from './templates/input'; import OutletTemplate from './templates/outlet'; import RootTemplate from './templates/root'; import OutletView from './views/outlet'; -export function setupApplicationRegistry(registry: Registry) { +export function setupApplicationRegistry(registry: Registry): void { registry.injection('renderer', 'env', '-environment:main'); // because we are using injections we can't use instantiate false @@ -46,7 +45,7 @@ export function setupApplicationRegistry(registry: Registry) { registry.injection('renderer', 'document', 'service:-document'); } -export function setupEngineRegistry(registry: Registry) { +export function setupEngineRegistry(registry: Registry): void { registry.optionsForType('template', { instantiate: false }); registry.register('view:-outlet', OutletView); @@ -61,8 +60,7 @@ export function setupEngineRegistry(registry: Registry) { registry.register('component:-checkbox', Checkbox); registry.register('component:link-to', LinkToComponent); - registry.register('component:input', Input); - registry.register('template:components/input', InputTemplate as any); + registry.register('component:input', InputComponent); registry.register('component:textarea', TextArea); diff --git a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts index bc28ca70b9d..21c866e5266 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts @@ -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 { @@ -27,6 +27,10 @@ interface CapturedBounds { lastNode: SimpleNode; } +function compileTemplate(templateSource: string, options: Partial) { + return compile(templateSource, options) as any; +} + type Expected = T | ((actual: T) => boolean); function isExpectedFunc(expected: Expected): expected is (actual: T) => boolean { @@ -301,7 +305,7 @@ if (ENV._DEBUG_RENDER_TREE) { super.init(...arguments); this.register( 'template:application', - compile( + compileTemplate( strip` {{#if @model}} @@ -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', }) ); @@ -337,7 +341,7 @@ if (ENV._DEBUG_RENDER_TREE) { super.init(...arguments); this.register( 'template:application', - compile( + compileTemplate( strip` {{#if @model}} @@ -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', }) ); @@ -682,7 +686,7 @@ if (ENV._DEBUG_RENDER_TREE) { super.init(...arguments); this.register( 'template:application', - compile( + compileTemplate( strip` {{outlet}} @@ -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('Hello {{@message}}', { + compileTemplate('Hello {{@message}}', { moduleName: 'foo/components/hello.hbs', }) ); @@ -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() ), }); diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/strict-mode-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/strict-mode-test.js new file mode 100644 index 00000000000..d6587280288 --- /dev/null +++ b/packages/@ember/-internals/glimmer/tests/integration/components/strict-mode-test.js @@ -0,0 +1,217 @@ +import { + moduleFor, + ApplicationTestCase, + RenderingTestCase, + defineComponent, + defineSimpleHelper, + defineSimpleModifier, +} from 'internal-test-helpers'; +import { EMBER_STRICT_MODE } from '@ember/canary-features'; + +import { + Input, + LinkComponent as LinkTo, + TextArea, + hash, + array, + concat, + get, + on, + fn, +} from '@ember/-internals/glimmer'; + +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 }, ''); + + this.registerComponent('bar', { ComponentClass: Bar }); + + this.render(''); + 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(''); + 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 }, '
'); + + this.registerComponent('bar', { ComponentClass: Bar }); + + this.render(''); + this.assertHTML('
Hello, world!
'); + 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(''); + 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(''); + 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(''); + this.assertHTML('foobar'); + this.assertStableRerender(); + } + } + ); + + moduleFor( + 'Strict Mode - built ins', + class extends RenderingTestCase { + '@test Can use Input'() { + let Foo = defineComponent({ Input }, ''); + + this.registerComponent('foo', { ComponentClass: Foo }); + + this.render(''); + this.assertComponentElement(this.firstChild, { + tagName: 'input', + attrs: { + type: 'text', + class: 'ember-text-field ember-view', + }, + }); + this.assertStableRerender(); + } + + '@skip Can use TextArea'() { + let Foo = defineComponent({ TextArea }, '