diff --git a/packages/@ember/-internals/glimmer/lib/helpers/-assert-implicit-component-helper-argument.ts b/packages/@ember/-internals/glimmer/lib/helpers/-assert-implicit-component-helper-argument.ts new file mode 100644 index 00000000000..4676f852644 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/helpers/-assert-implicit-component-helper-argument.ts @@ -0,0 +1,36 @@ +import { DEBUG } from '@glimmer/env'; +import { Opaque } from '@glimmer/interfaces'; +import { Tag, VersionedPathReference } from '@glimmer/reference'; +import { Arguments, Helper, VM } from '@glimmer/runtime'; +import { Maybe } from '@glimmer/util'; + +let helper: Maybe = undefined; + +if (DEBUG) { + class ComponentAssertionReference implements VersionedPathReference { + public tag: Tag; + + constructor(private component: VersionedPathReference, private message: string) { + this.tag = component.tag; + } + + value(): Opaque { + let value = this.component.value(); + + if (typeof value === 'string') { + throw new TypeError(this.message); + } + + return value; + } + + get(property: string): VersionedPathReference { + return this.component.get(property); + } + } + + helper = (_vm: VM, args: Arguments) => + new ComponentAssertionReference(args.positional.at(0), args.positional.at(1).value() as string); +} + +export default helper; diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 90632d6f432..754c2fc93fc 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -5,6 +5,7 @@ import { lookupComponent, lookupPartial, OwnedTemplateMeta } from '@ember/-inter import { EMBER_MODULE_UNIFICATION, GLIMMER_CUSTOM_COMPONENT_MANAGER } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { _instrumentStart } from '@ember/instrumentation'; +import { DEBUG } from '@glimmer/env'; import { ComponentDefinition, Opaque, @@ -18,6 +19,7 @@ import { CurlyComponentDefinition } from './component-managers/curly'; import { CustomManagerDefinition, ManagerDelegate } from './component-managers/custom'; import { TemplateOnlyComponentDefinition } from './component-managers/template-only'; import { isHelperFactory, isSimpleHelper } from './helper'; +import { default as componentAssertionHelper } from './helpers/-assert-implicit-component-helper-argument'; import { default as classHelper } from './helpers/-class'; import { default as htmlSafeHelper } from './helpers/-html-safe'; import { default as inputTypeHelper } from './helpers/-input-type'; @@ -76,6 +78,10 @@ const BUILTINS_HELPERS = { '-outlet': outletHelper, }; +if (DEBUG) { + BUILTINS_HELPERS['-assert-implicit-component-helper-argument'] = componentAssertionHelper; +} + const BUILTIN_MODIFIERS = { action: { manager: new ActionModifierManager(), state: null }, }; diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js index fa5ca224f10..fe00225bea6 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/contextual-components-test.js @@ -1387,6 +1387,98 @@ moduleFor( this.render('{{component (component "textarea" type="text")}}'); }, 'You cannot use `textarea` as a component name.'); } + + ['@test GH#17121 local variable should win over helper (without arguments)']() { + this.registerHelper('foo', () => 'foo helper'); + + this.registerComponent('foo-bar', { template: 'foo-bar component' }); + + this.render(strip` + {{#let (component 'foo-bar') as |foo|}} + {{foo}} + {{/let}} + `); + + this.assertText('foo-bar component'); + + this.assertStableRerender(); + } + + ['@test GH#17121 local variable should win over helper (with arguments)']() { + this.registerHelper('foo', params => `foo helper: ${params.join(' ')}`); + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend().reopenClass({ + positionalParams: 'params', + }), + template: 'foo-bar component:{{#each params as |param|}} {{param}}{{/each}}', + }); + + this.render(strip` + {{#let (component 'foo-bar') as |foo|}} + {{foo 1 2 3}} + {{/let}} + `); + + this.assertText('foo-bar component: 1 2 3'); + + this.assertStableRerender(); + } + + ['@test GH#17121 implicit component invocations should not perform string lookup'](assert) { + this.registerComponent('foo-bar', { template: 'foo-bar component' }); + + assert.throws( + () => + this.render(strip` + {{#let 'foo-bar' as |foo|}} + {{foo 1 2 3}} + {{/let}} + `), + new TypeError( + "expected `foo` to be a contextual component but found a string. Did you mean `(component foo)`? ('-top-level' @ L1:C29) " + ) + ); + } + + ['@test RFC#311 invoking named args (without arguments)']() { + this.registerComponent('x-outer', { template: '{{@inner}}' }); + this.registerComponent('x-inner', { template: 'inner' }); + + this.render('{{x-outer inner=(component "x-inner")}}'); + + this.assertText('inner'); + + this.assertStableRerender(); + } + + ['@test RFC#311 invoking named args (with arguments)']() { + this.registerComponent('x-outer', { template: '{{@inner 1 2 3}}' }); + + this.registerComponent('x-inner', { + ComponentClass: Component.extend().reopenClass({ + positionalParams: 'params', + }), + template: 'inner:{{#each params as |param|}} {{param}}{{/each}}', + }); + + this.render('{{x-outer inner=(component "x-inner")}}'); + + this.assertText('inner: 1 2 3'); + + this.assertStableRerender(); + } + + ['@test RFC#311 invoking named args (with a block)']() { + this.registerComponent('x-outer', { template: '{{#@inner}}outer{{/@inner}}' }); + this.registerComponent('x-inner', { template: 'inner {{yield}}' }); + + this.render('{{x-outer inner=(component "x-inner")}}'); + + this.assertText('inner outer'); + + this.assertStableRerender(); + } } ); diff --git a/packages/ember-template-compiler/lib/plugins/index.ts b/packages/ember-template-compiler/lib/plugins/index.ts index db5a90d864e..2089090076f 100644 --- a/packages/ember-template-compiler/lib/plugins/index.ts +++ b/packages/ember-template-compiler/lib/plugins/index.ts @@ -7,7 +7,7 @@ import DeprecateSendAction from './deprecate-send-action'; import TransformActionSyntax from './transform-action-syntax'; import TransformAngleBracketComponents from './transform-angle-bracket-components'; import TransformAttrsIntoArgs from './transform-attrs-into-args'; -import TransformDotComponentInvocation from './transform-dot-component-invocation'; +import TransformComponentInvocation from './transform-component-invocation'; import TransformEachInIntoEach from './transform-each-in-into-each'; import TransformHasBlockSyntax from './transform-has-block-syntax'; import TransformInElement from './transform-in-element'; @@ -23,7 +23,7 @@ import { ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax'; export type APluginFunc = (env: ASTPluginEnvironment) => ASTPlugin | undefined; const transforms: Array = [ - TransformDotComponentInvocation, + TransformComponentInvocation, TransformAngleBracketComponents, TransformTopLevelComponents, TransformInlineLinkTo, diff --git a/packages/ember-template-compiler/lib/plugins/transform-component-invocation.ts b/packages/ember-template-compiler/lib/plugins/transform-component-invocation.ts new file mode 100644 index 00000000000..f409d9cfe8c --- /dev/null +++ b/packages/ember-template-compiler/lib/plugins/transform-component-invocation.ts @@ -0,0 +1,243 @@ +import { DEBUG } from '@glimmer/env'; +import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax'; +import calculateLocationDisplay from '../system/calculate-location-display'; +import { Builders } from '../types'; + +/** + Transforms unambigious invocations of closure components to be wrapped with + the component helper. Once these syntaxes are fully supported by Glimmer VM + natively, this transform can be removed. + + ```handlebars + {{!-- this.foo is not a legal helper/component name --}} + {{this.foo "with" some="args"}} + ``` + + with + + ```handlebars + {{component this.foo "with" some="args"}} + ``` + + and + + ```handlebars + {{!-- this.foo is not a legal helper/component name --}} + {{#this.foo}}...{{/this.foo}} + ``` + + with + + ```handlebars + {{#component this.foo}}...{{/component}} + ``` + + and + + ```handlebars + {{!-- foo.bar is not a legal helper/component name --}} + {{foo.bar "with" some="args"}} + ``` + + with + + ```handlebars + {{component foo.bar "with" some="args"}} + ``` + + and + + ```handlebars + {{!-- foo.bar is not a legal helper/component name --}} + {{#foo.bar}}...{{/foo.bar}} + ``` + + with + + ```handlebars + {{#component foo.bar}}...{{/component}} + ``` + + and + + ```handlebars + {{!-- @foo is not a legal helper/component name --}} + {{@foo "with" some="args"}} + ``` + + with + + ```handlebars + {{component @foo "with" some="args"}} + ``` + + and + + ```handlebars + {{!-- @foo is not a legal helper/component name --}} + {{#@foo}}...{{/@foo}} + ``` + + with + + ```handlebars + {{#component @foo}}...{{/component}} + ``` + + and + + ```handlebars + {{#let ... as |foo|}} + {{!-- foo is a local variable --}} + {{foo "with" some="args"}} + {{/let}} + ``` + + with + + ```handlebars + {{#let ... as |foo|}} + {{component foo "with" some="args"}} + {{/let}} + ``` + + and + + ```handlebars + {{#let ... as |foo|}} + {{!-- foo is a local variable --}} + {{#foo}}...{{/foo}} + {{/let}} + ``` + + with + + ```handlebars + {{#let ... as |foo|}} + {{#component foo}}...{{/component}} + {{/let}} + ``` + + @private + @class TransFormComponentInvocation +*/ +export default function transformComponentInvocation(env: ASTPluginEnvironment): ASTPlugin { + let { moduleName } = env.meta; + let { builders: b } = env.syntax; + let locals: string[][] = []; + + return { + name: 'transform-component-invocation', + + visitor: { + ElementNode: { + enter(node: AST.ElementNode) { + locals.push(node.blockParams); + }, + + exit() { + locals.pop(); + }, + }, + + BlockStatement: { + enter(node: AST.BlockStatement) { + // check this before we push the new locals + if (isBlockInvocation(node, locals)) { + wrapInComponent(moduleName, node, b); + } + + locals.push(node.program.blockParams); + }, + + exit() { + locals.pop(); + }, + }, + + MustacheStatement(node: AST.MustacheStatement): AST.Node | void { + if (isInlineInvocation(node, locals)) { + wrapInComponent(moduleName, node, b); + } + }, + }, + }; +} + +function isInlineInvocation(node: AST.MustacheStatement, locals: string[][]): boolean { + let { path } = node; + return isPath(path) && isIllegalName(path, locals) && hasArguments(node); +} + +function isPath(node: AST.PathExpression | AST.Literal): node is AST.PathExpression { + return node.type === 'PathExpression'; +} + +function isIllegalName(node: AST.PathExpression, locals: string[][]): boolean { + return isThisPath(node) || isDotPath(node) || isNamedArg(node) || isLocalVariable(node, locals); +} + +function isThisPath(node: AST.PathExpression): boolean { + return node.this === true; +} + +function isDotPath(node: AST.PathExpression): boolean { + return node.parts.length > 1; +} + +function isNamedArg(node: AST.PathExpression): boolean { + return node.data === true; +} + +function isLocalVariable(node: AST.PathExpression, locals: string[][]): boolean { + return !node.this && hasLocalVariable(node.parts[0], locals); +} + +function hasLocalVariable(name: string, locals: string[][]): boolean { + return locals.some(names => names.indexOf(name) !== -1); +} + +function hasArguments(node: AST.MustacheStatement): boolean { + return node.params.length > 0 || node.hash.pairs.length > 0; +} + +function isBlockInvocation(node: AST.BlockStatement, locals: string[][]): boolean { + return isIllegalName(node.path, locals); +} + +let wrapInAssertion: ( + moduleName: string, + node: AST.PathExpression, + builder: Builders +) => AST.Expression; + +if (DEBUG) { + wrapInAssertion = (moduleName, node, b) => { + let error = b.string( + `expected \`${ + node.original + }\` to be a contextual component but found a string. Did you mean \`(component ${ + node.original + })\`? ${calculateLocationDisplay(moduleName, node.loc)}` + ); + + return b.sexpr( + b.path('-assert-implicit-component-helper-argument'), + [node, error], + b.hash(), + node.loc + ); + }; +} else { + wrapInAssertion = (_, node) => node; +} + +function wrapInComponent( + moduleName: string, + node: AST.MustacheStatement | AST.BlockStatement, + b: Builders +) { + let component = wrapInAssertion(moduleName, node.path as AST.PathExpression, b); + node.path = b.path('component'); + node.params.unshift(component); +} diff --git a/packages/ember-template-compiler/lib/plugins/transform-dot-component-invocation.ts b/packages/ember-template-compiler/lib/plugins/transform-dot-component-invocation.ts deleted file mode 100644 index bf433d19906..00000000000 --- a/packages/ember-template-compiler/lib/plugins/transform-dot-component-invocation.ts +++ /dev/null @@ -1,101 +0,0 @@ -import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax'; -import { Builders } from '../types'; - -/** - Transforms dot invocation of closure components to be wrapped - with the component helper. This allows for a more static invocation - of the component. - - ```handlebars - {{#my-component as |comps|}} - {{comp.dropdown isOpen=false}} - {{/my-component}} - ``` - - with - - ```handlebars - {{#my-component as |comps|}} - {{component comp.dropdown isOpen=false}} - {{/my-component}} - ``` - and - - ```handlebars - {{#my-component as |comps|}} - {{comp.dropdown isOpen}} - {{/my-component}} - ``` - - with - - ```handlebars - {{#my-component as |comps|}} - {{component comp.dropdown isOpen}} - {{/my-component}} - ``` - - and - - ```handlebars - {{#my-component as |comps|}} - {{#comp.dropdown}}Open{{/comp.dropdown}} - {{/my-component}} - ``` - - with - - ```handlebars - {{#my-component as |comps|}} - {{#component comp.dropdown}}Open{{/component}} - {{/my-component}} - ``` - - @private - @class TransFormDotComponentInvocation -*/ -export default function transformDotComponentInvocation(env: ASTPluginEnvironment): ASTPlugin { - let { builders: b } = env.syntax; - - return { - name: 'transform-dot-component-invocation', - - visitor: { - MustacheStatement(node: AST.MustacheStatement): AST.Node | void { - if (isInlineInvocation(node.path, node.params, node.hash)) { - wrapInComponent(node, b); - } - }, - BlockStatement(node: AST.BlockStatement): AST.Node | void { - if (isMultipartPath(node.path)) { - wrapInComponent(node, b); - } - }, - }, - }; -} - -function isMultipartPath(path: AST.PathExpression | AST.Literal) { - return (path as AST.PathExpression).parts && (path as AST.PathExpression).parts.length > 1; -} - -function isInlineInvocation( - path: AST.PathExpression | AST.Literal, - params: AST.Node[], - hash: AST.Hash -) { - if (isMultipartPath(path)) { - if (params.length > 0 || hash.pairs.length > 0) { - return true; - } - } - - return false; -} - -function wrapInComponent(node: AST.MustacheStatement | AST.BlockStatement, builder: Builders) { - let component = node.path; - let componentHelper = builder.path('component'); - node.path = componentHelper; - node.params.unshift(component); -} diff --git a/packages/ember-template-compiler/tests/plugins/transform-dot-component-invocation-test.js b/packages/ember-template-compiler/tests/plugins/transform-component-invocation-test.js similarity index 54% rename from packages/ember-template-compiler/tests/plugins/transform-dot-component-invocation-test.js rename to packages/ember-template-compiler/tests/plugins/transform-component-invocation-test.js index e017c88ec7e..c91857cb088 100644 --- a/packages/ember-template-compiler/tests/plugins/transform-dot-component-invocation-test.js +++ b/packages/ember-template-compiler/tests/plugins/transform-component-invocation-test.js @@ -2,21 +2,24 @@ import { compile } from '../../index'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; moduleFor( - 'ember-template-compiler: transforms dot component invocation', + 'ember-template-compiler: transforms component invocation', class extends AbstractTestCase { - ['@test Does not throw a compiler error for path components'](assert) { + ['@test Does not throw a compiler error for component invocations'](assert) { assert.expect(0); [ '{{this.modal open}}', '{{this.modal isOpen=true}}', '{{#this.modal}}Woot{{/this.modal}}', + '{{@modal open}}', // RFC#311 + '{{@modal isOpen=true}}', // RFC#311 + '{{#@modal}}Woot{{/@modal}}', // RFC#311 '{{c.modal open}}', '{{c.modal isOpen=true}}', '{{#c.modal}}Woot{{/c.modal}}', - '{{#my-component as |c|}}{{c.a name="Chad"}}{{/my-component}}', - '{{#my-component as |c|}}{{c.a "Chad"}}{{/my-component}}', - '{{#my-component as |c|}}{{#c.a}}{{/c.a}}{{/my-component}}', + '{{#my-component as |c|}}{{c name="Chad"}}{{/my-component}}', // RFC#311 + '{{#my-component as |c|}}{{c "Chad"}}{{/my-component}}', // RFC#311 + '{{#my-component as |c|}}{{#c}}{{/c}}{{/my-component}}', // RFC#311 '', // GH#15740 '', // GH#15217 ].forEach((layout, i) => {