Skip to content

Commit

Permalink
Make helpers unambiguious with parens aka subexpressions
Browse files Browse the repository at this point in the history
  • Loading branch information
@lolmaus
lolmaus committed Dec 4, 2023
1 parent 821a44d commit 7ca586f
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 38 deletions.
2 changes: 0 additions & 2 deletions transforms/angle-brackets/known-helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ const KNOWN_HELPERS = [
'loc',
'log',
'mut',
'outlet',
'partial',
'query-params',
'readonly',
'unbound',
'unless',
'with',
'yield',

// Glimmer VM
'identity', // glimmer blocks
Expand Down
3 changes: 3 additions & 0 deletions transforms/angle-brackets/telemetry/mock-invokables.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,7 @@ module.exports = {
'app/helpers/nested/helper': {
type: 'Helper',
},
'app/helpers/fooknownhelper': {
type: 'Helper',
},
};
45 changes: 30 additions & 15 deletions transforms/angle-brackets/transform.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ function getNonDataAttributesFromParams(params) {
return params.filter((it) => !isDataAttrPathExpression(it));
}

function shouldIgnoreMustacheStatement(fullName, config, invokableData) {
function isKnownHelper(fullName, config, invokableData) {
let { helpers, components } = invokableData;
let isTelemetryData = !!(helpers || components);

Expand All @@ -337,11 +337,18 @@ function shouldIgnoreMustacheStatement(fullName, config, invokableData) {
}

if (isTelemetryData) {
let isComponent =
!config.helpers.includes(name) &&
[...(components || []), ...BUILT_IN_COMPONENTS].includes(name);

if (isComponent) {
return false;
}

let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])];
let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name);
let isComponent = [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
let strName = `${name}`; // coerce boolean and number to string
return (isHelper || !isComponent) && !strName.includes('.');
return isHelper && !strName.includes('.');
} else {
return KNOWN_HELPERS.includes(name) || config.helpers.includes(name);
}
Expand All @@ -363,7 +370,7 @@ function nodeHasPositionalParameters(node) {
return false;
}

function transformNode(node, fileInfo, config) {
function transformComponentNode(node, fileInfo, config) {
if (
hasValuelessDataParams(node.params) &&
shouldSkipDataTestParams(node.params, config.includeValuelessDataTestAttributes)
Expand Down Expand Up @@ -425,6 +432,10 @@ function transformNode(node, fileInfo, config) {
);
}

function transformHelperNode(node) {
return b.mustache(b.sexpr(node.path, node.params, node.hash));
}

function subExpressionToMustacheStatement(subExpression) {
return b.mustache(subExpression.path, subExpression.params, subExpression.hash);
}
Expand Down Expand Up @@ -457,34 +468,38 @@ function transformToAngleBracket(fileInfo, config, invokableData) {
* Transform the attributes names & values properly
*/
return {
MustacheStatement(node) {
MustacheStatement(node, walkerPath) {
const tagName = `${node.path && node.path.original}`;

if (config.components && !config.components.includes(tagName)) return;

const isTagKnownHelper = isKnownHelper(tagName, config, invokableData);

// Don't change attribute statements
const isValidMustache =
node.loc.source !== '(synthetic)' &&
!shouldIgnoreMustacheStatement(tagName, config, invokableData);
const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper;
const isNestedComponent = isNestedComponentTagName(tagName);

if (
isValidMustache &&
isValidMustacheComponent &&
(node.hash.pairs.length > 0 || node.params.length > 0 || isNestedComponent)
) {
return transformNode(node, fileInfo, config);
return transformComponentNode(node, fileInfo, config);
} else if (
isTagKnownHelper &&
node.path.type !== 'SubExpression' &&
walkerPath.parent.node.type !== 'AttrNode' &&
walkerPath.parent.node.type !== 'ConcatStatement'
) {
return transformHelperNode(node, walkerPath);
}
},
BlockStatement(node) {
let tagName = `${node.path.original}`;

if (config.components && !config.components.includes(tagName)) return;

if (
!shouldIgnoreMustacheStatement(node.path.original, config, invokableData) ||
isWallStreet(tagName)
) {
return transformNode(node, fileInfo, config);
if (!isKnownHelper(node.path.original, config, invokableData) || isWallStreet(tagName)) {
return transformComponentNode(node, fileInfo, config);
}
},
AttrNode: {
Expand Down
77 changes: 56 additions & 21 deletions transforms/angle-brackets/transform.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ test('deeply-nested-sub', () => {
)
}}
`;

/**
* NOTE: An issue has been opened in `ember-template-recast` (https://github.com/ember-template-lint/ember-template-recast/issues/82)
* regarding to create an API to allow a transform to customize the whitespace for newly created nodes.
Expand Down Expand Up @@ -477,7 +476,7 @@ test('let', () => {
{{#let (capitalize this.person.firstName) (capitalize this.person.lastName)
as |firstName lastName|
}}
Welcome back {{concat firstName ' ' lastName}}
Welcome back {{(concat firstName ' ' lastName)}}
Account Details:
First Name: {{firstName}}
Expand Down Expand Up @@ -555,8 +554,8 @@ test('link-to-inline', () => {
<LinkTo @route=\\"apps.segments\\" class=\\"tabs__discrete-tab\\" @activeClass=\\"o__selected\\" @current-when=\\"apps.segments\\" data-test-segment-link=\\"segments\\">Segments</LinkTo>
<LinkTo @route={{this.dynamicPath}} class=\\"tabs__discrete-tab\\" @activeClass=\\"o__selected\\" @current-when=\\"apps.segments\\" data-test-segment-link=\\"segments\\">Segments</LinkTo>
<LinkTo @route=\\"apps.app.companies.segments.segment\\" @model={{segment}} class=\\"t__em-link\\">{{segment.name}}</LinkTo>
<LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{t \\"show\\"}}</LinkTo>
<LinkTo @route=\\"user\\" @model={{if linkActor event.actor.id event.user.id}}>{{t \\"show\\"}}</LinkTo>
<LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{(t \\"show\\")}}</LinkTo>
<LinkTo @route=\\"user\\" @model={{if linkActor event.actor.id event.user.id}}>{{(t \\"show\\")}}</LinkTo>
<LinkTo @route=\\"user\\" @models={{array this.first this.second}}>Show</LinkTo>
<LinkTo @route=\\"user\\" @models={{array this.first this.second}} @query={{hash foo=\\"baz\\"}}>Show</LinkTo>
<LinkTo @route=\\"user\\" @model={{this.first}}>Show</LinkTo>
Expand Down Expand Up @@ -840,7 +839,7 @@ test('t-helper', () => {

expect(runTest('t-helper.hbs', input)).toMatchInlineSnapshot(`
"
{{t \\"some.string\\" param=\\"string\\" another=1}}
{{(t \\"some.string\\" param=\\"string\\" another=1)}}
"
`);
});
Expand Down Expand Up @@ -978,7 +977,7 @@ test('skip-default-helpers', () => {
expect(runTest('skip-default-helpers.hbs', input, options)).toMatchInlineSnapshot(`
"
<div id=\\"main-container\\">
{{liquid-outlet}}
{{(liquid-outlet)}}
</div>
<button {{action \\"toggle\\" \\"showA\\"}}>
Toggle A/B</button>
Expand All @@ -998,11 +997,11 @@ test('skip-default-helpers', () => {
Two
</div>
{{/liquid-if}}
{{moment '12-25-1995' 'MM-DD-YYYY'}}
{{moment-from '1995-12-25' '2995-12-25' hideAffix=true}}
{{(moment '12-25-1995' 'MM-DD-YYYY')}}
{{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}}
<SomeComponent @foo={{true}} />
{{some-helper1 foo=true}}
{{some-helper2 foo=true}}
{{(some-helper1 foo=true)}}
{{(some-helper2 foo=true)}}
"
`);
});
Expand Down Expand Up @@ -1040,7 +1039,7 @@ test('skip-default-helpers (no-config)', () => {
expect(runTest('skip-default-helpers.hbs', input)).toMatchInlineSnapshot(`
"
<div id=\\"main-container\\">
{{liquid-outlet}}
{{(liquid-outlet)}}
</div>
<button {{action \\"toggle\\" \\"showA\\"}}>
Toggle A/B</button>
Expand All @@ -1060,8 +1059,8 @@ test('skip-default-helpers (no-config)', () => {
Two
</div>
{{/liquid-if}}
{{moment '12-25-1995' 'MM-DD-YYYY'}}
{{moment-from '1995-12-25' '2995-12-25' hideAffix=true}}
{{(moment '12-25-1995' 'MM-DD-YYYY')}}
{{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}}
<SomeComponent @foo={{true}} />
<SomeHelper1 @foo={{true}} />
<SomeHelper2 @foo={{true}} />
Expand All @@ -1087,8 +1086,8 @@ test('custom-options', () => {
expect(runTest('custom-options.hbs', input, options)).toMatchInlineSnapshot(`
"
<SomeComponent @foo={{true}} />
{{some-helper1 foo=true}}
{{some-helper2 foo=true}}
{{(some-helper1 foo=true)}}
{{(some-helper2 foo=true)}}
{{link-to \\"Title\\" \\"some.route\\"}}
{{textarea value=this.model.body}}
{{input type=\\"checkbox\\" name=\\"email-opt-in\\" checked=this.model.emailPreference}}
Expand Down Expand Up @@ -1181,7 +1180,7 @@ test('preserve arguments', () => {
"
<FooBar @class=\\"baz\\" />
{{foo-bar data-baz class=\\"baz\\"}}
<LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{t \\"show\\"}}</LinkTo>
<LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{(t \\"show\\")}}</LinkTo>
"
`);
});
Expand Down Expand Up @@ -1278,18 +1277,34 @@ test('wallstreet-telemetry', () => {
let input = `
{{nested$helper}}
{{nested::helper}}
{{nested$helper param="cool!"}}
{{nested::helper param="yeah!"}}
{{helper-1}}
`;

expect(runTest('wallstreet-telemetry.hbs', input)).toMatchInlineSnapshot(`
"
{{nested$helper}}
{{nested::helper}}
{{nested$helper param=\\"cool!\\"}}
{{nested::helper param=\\"yeah!\\"}}
{{helper-1}}
{{(nested::helper)}}
{{(nested::helper param=\\"yeah!\\")}}
{{(helper-1)}}
"
`);
});

test('wrapping-helpers-with-parens', () => {
let input = `
{{fooknownhelper}}
{{(fooknownhelper)}}
{{fooknownhelper data-test-foo foo="bar"}}
{{foounknownhelper}}
`;

expect(runTest('wrapping-helpers-with-parens.hbs', input)).toMatchInlineSnapshot(`
"
{{(fooknownhelper)}}
{{(fooknownhelper)}}
{{(fooknownhelper data-test-foo foo=\\"bar\\")}}
{{foounknownhelper}}
"
`);
});
Expand Down Expand Up @@ -1335,3 +1350,23 @@ test('No telemetry', () => {
"
`);
});

test('pipe', () => {
let input = `<Foo @title={{concat ">"}} as |bar|></Foo>`;

expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(
`"<Foo @title={{concat \\">\\"}} as |bar|></Foo>"`
);
});

test('outlet', () => {
let input = `{{outlet}}`;

expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(`"{{outlet}}"`);
});

test('yield', () => {
let input = `{{yield}}`;

expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(`"{{yield}}"`);
});

0 comments on commit 7ca586f

Please sign in to comment.