diff --git a/packages/@ember/-internals/glimmer/lib/helpers/-disallow-dynamic-resolution.ts b/packages/@ember/-internals/glimmer/lib/helpers/-disallow-dynamic-resolution.ts new file mode 100644 index 00000000000..7d761c5c768 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/helpers/-disallow-dynamic-resolution.ts @@ -0,0 +1,57 @@ +/** +@module ember +*/ +import { assert } from '@ember/debug'; +import { CapturedArguments } from '@glimmer/interfaces'; +import { createComputeRef, valueForRef } from '@glimmer/reference'; +import { internalHelper } from './internal-helper'; + +export default internalHelper(({ positional, named }: CapturedArguments) => { + assert( + `[BUG] wrong number of positional arguments, expecting 1, got ${positional.length}`, + positional.length === 1 + ); + + let nameOrValueRef = positional[0]; + + assert(`[BUG] expecting \`type\` named argument`, 'type' in named); + assert(`[BUG] expecting \`loc\` named argument`, 'loc' in named); + assert(`[BUG] expecting \`original\` named argument`, 'original' in named); + + let typeRef = named.type; + let locRef = named.loc; + let originalRef = named.original; + + // Bug: why do these fail? + // assert('[BUG] expecting a string literal for the `type` argument', isConstRef(typeRef)); + // assert('[BUG] expecting a string literal for the `loc` argument', isConstRef(locRef)); + // assert('[BUG] expecting a string literal for the `original` argument', isConstRef(originalRef)); + + const type = valueForRef(typeRef); + const loc = valueForRef(locRef); + const original = valueForRef(originalRef); + + assert('[BUG] expecting a string literal for the `type` argument', typeof type === 'string'); + assert('[BUG] expecting a string literal for the `loc` argument', typeof loc === 'string'); + assert( + '[BUG] expecting a string literal for the `original` argument', + typeof original === 'string' + ); + + return createComputeRef(() => { + let nameOrValue = valueForRef(nameOrValueRef); + + assert( + `Passing a dynamic string to the \`(${type})\` keyword is disallowed. ` + + `(You specified \`(${type} ${original})\` and \`${original}\` evaluated into "${nameOrValue}".) ` + + `This ensures we can statically analyze the template and determine which ${type}s are used. ` + + `If the ${type} name is always the same, use a string literal instead, i.e. \`(${type} "${nameOrValue}")\`. ` + + `Otherwise, import the ${type}s into JavaScript and pass them to the ${type} keyword. ` + + 'See https://github.com/emberjs/rfcs/blob/master/text/0496-handlebars-strict-mode.md#4-no-dynamic-resolution for details. ' + + loc, + typeof nameOrValue !== 'string' + ); + + return nameOrValue; + }); +}); diff --git a/packages/@ember/-internals/glimmer/lib/helpers/-resolve.ts b/packages/@ember/-internals/glimmer/lib/helpers/-resolve.ts new file mode 100644 index 00000000000..34eda61c3c2 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/helpers/-resolve.ts @@ -0,0 +1,39 @@ +/** +@module ember +*/ +import { Owner } from '@ember/-internals/owner'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { CapturedArguments } from '@glimmer/interfaces'; +import { createConstRef, isConstRef, valueForRef } from '@glimmer/reference'; +import { internalHelper } from './internal-helper'; + +export default internalHelper(({ positional }: CapturedArguments, owner: Owner | undefined) => { + // why is this allowed to be undefined in the first place? + assert('[BUG] missing owner', owner); + + assert( + `[BUG] wrong number of positional arguments, expecting 1, got ${positional.length}`, + positional.length === 1 + ); + + let fullNameRef = positional[0]; + + assert('[BUG] expecting a string literal as argument', isConstRef(fullNameRef)); + + let fullName = valueForRef(fullNameRef); + + assert('[BUG] expecting a string literal as argument', typeof fullName === 'string'); + assert('[BUG] expecting a valid full name', fullName.split(':').length === 2); + + if (DEBUG) { + let [type, name] = fullName.split(':'); + + assert( + `Attempted to invoke \`(-resolve "${fullName}")\`, but ${name} was not a valid ${type} name.`, + owner.hasRegistration(fullName) + ); + } + + return createConstRef(owner.factoryFor(fullName)?.class, `(-resolve "${fullName}")`); +}); diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 8c87ec67c64..c20abe47c5f 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -42,8 +42,10 @@ import { isClassicHelper, SimpleHelper, } from './helper'; +import { default as disallowDynamicResolution } from './helpers/-disallow-dynamic-resolution'; import { default as inElementNullCheckHelper } from './helpers/-in-element-null-check'; import { default as normalizeClassHelper } from './helpers/-normalize-class'; +import { default as resolve } from './helpers/-resolve'; import { default as trackArray } from './helpers/-track-array'; import { default as action } from './helpers/action'; import { default as eachIn } from './helpers/each-in'; @@ -181,12 +183,27 @@ const BUILTIN_KEYWORD_HELPERS = { '-hash': hash, '-each-in': eachIn, '-normalize-class': normalizeClassHelper, + '-resolve': resolve, '-track-array': trackArray, '-mount': mountHelper, '-outlet': outletHelper, '-in-el-null': inElementNullCheckHelper, }; +if (DEBUG) { + BUILTIN_KEYWORD_HELPERS['-disallow-dynamic-resolution'] = disallowDynamicResolution; +} else { + // Bug: this may be a quirk of our test setup? + // In prod builds, this is a no-op helper and is unused in practice. We shouldn't need + // to add it at all, but the current test build doesn't produce a "prod compiler", so + // we ended up running the debug-build for the template compliler in prod tests. Once + // that is fixed, this can be removed. For now, this allows the test to work and does + // not really harm anything, since it's just a no-op pass-through helper and the bytes + // has to be included anyway. In the future, perhaps we can avoid the latter by using + // `import(...)`? + BUILTIN_KEYWORD_HELPERS['-disallow-dynamic-resolution'] = disallowDynamicResolution; +} + const BUILTIN_HELPERS = { ...BUILTIN_KEYWORD_HELPERS, array, 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 dd92643ce33..508dfc567ed 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 @@ -581,6 +581,45 @@ moduleFor( assert.equal(updateCount, 1); } + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can resolve a modifier'() { + this.registerModifier( + 'replace', + defineSimpleModifier((element, [text]) => (element.innerHTML = text ?? 'Hello, world!')) + ); + + // BUG: this should work according to the RFC + // this.render( + // '[
Nope
][
Nope
]' + // ); + this.render( + '[
Nope
][
Nope
]' + ); + this.assertText('[Hello, world!][wow]'); + this.assertStableRerender(); + } + + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Cannot dynamically resolve a modifier'(assert) { + this.registerModifier( + 'replace', + defineSimpleModifier((element) => (element.innerHTML = 'Hello, world!')) + ); + + if (DEBUG) { + expectAssertion( + () => + this.render( + // BUG: this should work according to the RFC + // '
Nope
', + '
Nope
', + { name: 'replace' } + ), + /Passing a dynamic string to the `\(modifier\)` keyword is disallowed\./ + ); + } else { + assert.expect(0); + } + } + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can be curried'() { let val = defineSimpleModifier((element, [text]) => (element.innerHTML = text)); diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/custom-helper-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/custom-helper-test.js index 08d55ef94b9..ba4bb014444 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/helpers/custom-helper-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/helpers/custom-helper-test.js @@ -780,6 +780,27 @@ moduleFor( }, expectedMessage); } + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can resolve a helper'() { + this.registerHelper('hello-world', ([text]) => text ?? 'Hello, world!'); + + this.render('[{{helper "hello-world"}}][{{helper (helper "hello-world") "wow"}}]'); + this.assertText('[Hello, world!][wow]'); + this.assertStableRerender(); + } + + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Cannot dynamically resolve a helper'(assert) { + this.registerHelper('hello-world', () => 'Hello, world!'); + + if (DEBUG) { + expectAssertion( + () => this.render('{{helper this.name}}', { name: 'hello-world' }), + /Passing a dynamic string to the `\(helper\)` keyword is disallowed\./ + ); + } else { + assert.expect(0); + } + } + '@feature(EMBER_DYNAMIC_HELPERS_AND_MODIFIERS) Can use a curried dynamic helper'() { let val = defineSimpleHelper((value) => value); diff --git a/packages/ember-template-compiler/lib/plugins/index.ts b/packages/ember-template-compiler/lib/plugins/index.ts index 4b8ac4fe831..2efa104176a 100644 --- a/packages/ember-template-compiler/lib/plugins/index.ts +++ b/packages/ember-template-compiler/lib/plugins/index.ts @@ -14,6 +14,7 @@ import TransformInElement from './transform-in-element'; import TransformLinkTo from './transform-link-to'; import TransformOldClassBindingSyntax from './transform-old-class-binding-syntax'; import TransformQuotedBindingsIntoJustBindings from './transform-quoted-bindings-into-just-bindings'; +import TransformResolutions from './transform-resolutions'; import TransformWrapMountAndOutlet from './transform-wrap-mount-and-outlet'; import { EMBER_DYNAMIC_HELPERS_AND_MODIFIERS, EMBER_NAMED_BLOCKS } from '@ember/canary-features'; @@ -38,7 +39,9 @@ export const RESOLUTION_MODE_TRANSFORMS = Object.freeze( DeprecateWith, SEND_ACTION ? DeprecateSendAction : null, !EMBER_NAMED_BLOCKS ? AssertAgainstNamedBlocks : null, - !EMBER_DYNAMIC_HELPERS_AND_MODIFIERS ? AssertAgainstDynamicHelpersModifiers : null, + EMBER_DYNAMIC_HELPERS_AND_MODIFIERS + ? TransformResolutions + : AssertAgainstDynamicHelpersModifiers, ].filter(notNull) ); diff --git a/packages/ember-template-compiler/lib/plugins/transform-resolutions.ts b/packages/ember-template-compiler/lib/plugins/transform-resolutions.ts new file mode 100644 index 00000000000..14f3849f1ce --- /dev/null +++ b/packages/ember-template-compiler/lib/plugins/transform-resolutions.ts @@ -0,0 +1,198 @@ +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { AST, ASTPlugin, print } from '@glimmer/syntax'; +import calculateLocationDisplay from '../system/calculate-location-display'; +import { EmberASTPluginEnvironment } from '../types'; +import { isPath, isStringLiteral, trackLocals } from './utils'; + +/** + @module ember +*/ + +/** + A Glimmer2 AST transformation that replaces all instances of + + ```handlebars + {{helper "..." ...}} + ``` + + with + + ```handlebars + {{helper (-resolve "helper:...") ...}} + ``` + + and + + ```handlebars + {{helper ... ...}} + ``` + + with + + ```handlebars + {{helper (-disallow-dynamic-resolution ...) ...}} + ``` + + and + + ```handlebars + {{modifier "..." ...}} + ``` + + with + + ```handlebars + {{modifier (-resolve "modifier:...") ...}} + ``` + and + + ```handlebars + {{modifier ... ...}} + ``` + + with + + ```handlebars + {{modifier (-disallow-dynamic-resolution ...) ...}} + ``` + + @private + @class TransformResolutions +*/ + +const TARGETS = Object.freeze(['helper', 'modifier']); + +export default function transformResolutions(env: EmberASTPluginEnvironment): ASTPlugin { + let { builders: b } = env.syntax; + let { moduleName } = env.meta; + let { hasLocal, node: tracker } = trackLocals(); + let seen: Set | undefined; + + return { + name: 'transform-resolutions', + + visitor: { + Template: { + enter() { + seen = new Set(); + }, + + exit() { + seen = undefined; + }, + }, + + Block: tracker, + + ElementNode: { + keys: { + children: tracker, + }, + }, + + MustacheStatement(node: AST.MustacheStatement): AST.Node | void { + assert('[BUG] seen set should be available', seen); + + if (seen.has(node)) { + return; + } + + if ( + isPath(node.path) && + !isLocalVariable(node.path, hasLocal) && + TARGETS.indexOf(node.path.original) !== -1 + ) { + let result = b.mustache( + node.path, + transformParams(b, node.params, node.path.original, moduleName, node.loc), + node.hash, + node.trusting, + node.loc, + node.strip + ); + + // Avoid double/infinite-processing + seen.add(result); + + return result; + } + }, + SubExpression(node: AST.SubExpression): AST.Node | void { + assert('[BUG] seen set should be available', seen); + + if (seen.has(node)) { + return; + } + + if ( + isPath(node.path) && + !isLocalVariable(node.path, hasLocal) && + TARGETS.indexOf(node.path.original) !== -1 + ) { + let result = b.sexpr( + node.path, + transformParams(b, node.params, node.path.original, moduleName, node.loc), + node.hash, + node.loc + ); + + // Avoid double/infinite-processing + seen.add(result); + + return result; + } + }, + }, + }; +} + +function isLocalVariable(node: AST.PathExpression, hasLocal: (k: string) => boolean): boolean { + return !node.this && node.parts.length === 1 && hasLocal(node.parts[0]); +} + +function transformParams( + b: EmberASTPluginEnvironment['syntax']['builders'], + params: AST.Expression[], + type: string, + moduleName: string, + loc: AST.SourceLocation | undefined +): AST.Expression[] { + let [first, ...rest] = params; + + assert( + `The ${type} keyword requires at least one positional arguments ${calculateLocationDisplay( + moduleName, + loc + )}`, + first + ); + + if (isStringLiteral(first)) { + return [ + b.sexpr( + b.path('-resolve', first.loc), + [b.string(`${type}:${first.value}`)], + undefined, + first.loc + ), + ...rest, + ]; + } else if (DEBUG) { + return [ + b.sexpr( + b.path('-disallow-dynamic-resolution', first.loc), + [first], + b.hash([ + b.pair('type', b.string(type), first.loc), + b.pair('loc', b.string(calculateLocationDisplay(moduleName, loc)), first.loc), + b.pair('original', b.string(print(first))), + ]), + first.loc + ), + ...rest, + ]; + } else { + return params; + } +} diff --git a/packages/ember-template-compiler/lib/plugins/utils.ts b/packages/ember-template-compiler/lib/plugins/utils.ts index fa310f00e31..a2e42b39ca0 100644 --- a/packages/ember-template-compiler/lib/plugins/utils.ts +++ b/packages/ember-template-compiler/lib/plugins/utils.ts @@ -8,18 +8,22 @@ export function isSubExpression(node: AST.Node): node is AST.SubExpression { return node.type === 'SubExpression'; } +export function isStringLiteral(node: AST.Expression): node is AST.StringLiteral { + return node.type === 'StringLiteral'; +} + export function trackLocals() { let locals = new Map(); let node = { - enter(node: AST.Program | AST.ElementNode) { + enter(node: AST.Program | AST.Block | AST.ElementNode) { for (let param of node.blockParams) { let value = locals.get(param) || 0; locals.set(param, value + 1); } }, - exit(node: AST.Program | AST.ElementNode) { + exit(node: AST.Program | AST.Block | AST.ElementNode) { for (let param of node.blockParams) { let value = locals.get(param) - 1;