Skip to content

Commit 9da3171

Browse files
patricklxpatricklx
committed
pass correct scope
1 parent 209b81e commit 9da3171

File tree

4 files changed

+126
-49
lines changed

4 files changed

+126
-49
lines changed

‎package.json

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
"scripts": {
99
"prepare": "tsc",
1010
"build": "tsc",
11-
"pretest": "tsc",
1211
"lint": "eslint --cache --ext .ts .",
1312
"test": "vitest --run",
1413
"clean": "git clean -d -f -x src __tests__"

‎src/plugin.ts

+96-34
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ import { JSUtils, ExtendedPluginBuilder } from './js-utils';
77
import type { EmberTemplateCompiler, PreprocessOptions } from './ember-template-compiler';
88
import { LegacyModuleName } from './public-types';
99
import { ScopeLocals } from './scope-locals';
10-
import { ASTPluginBuilder, getTemplateLocals, preprocess, print } from '@glimmer/syntax';
10+
import {
11+
ASTPluginBuilder,
12+
ASTPluginEnvironment,
13+
NodeVisitor,
14+
preprocess,
15+
print,
16+
} from '@glimmer/syntax';
1117

1218
export * from './public-types';
1319

@@ -372,33 +378,80 @@ function runtimeErrorIIFE(babel: typeof Babel, replacements: { ERROR_MESSAGE: st
372378

373379
function buildScopeLocals(
374380
userTypedOptions: Record<string, unknown>,
375-
formatOptions: ModuleConfig,
376-
templateContent: string
381+
formatOptions: ModuleConfig
377382
): ScopeLocals {
378383
if (formatOptions.rfc931Support && userTypedOptions.eval) {
379-
return discoverLocals(templateContent);
384+
return new ScopeLocals();
380385
} else if (userTypedOptions.scope) {
381386
return userTypedOptions.scope as ScopeLocals;
382387
} else {
383388
return new ScopeLocals();
384389
}
385390
}
386391

387-
function discoverLocals(templateContent: string): ScopeLocals {
388-
// this is wrong, but the right thing is unreleased in
389-
// https://github.com/glimmerjs/glimmer-vm/pull/1421, so for the moment I'm
390-
// sticking with the exact behavior that ember-templates-imports has.
391-
//
392-
// (the reason it's wrong is that the correct answer depends on not just the
393-
// template, but the ambient javascript scope. Anything in locals needs to win
394-
// over ember keywords. Otherwise we can never introduce new keywords.)
395-
let scopeLocals = new ScopeLocals();
396-
for (let local of getTemplateLocals(templateContent)) {
397-
if (local.match(/^[$A-Z_][0-9A-Z_$]*$/i)) {
398-
scopeLocals.add(local);
392+
function discoverLocals<EnvSpecificOptions>(
393+
jsPath: NodePath<t.Expression>,
394+
state: State<EnvSpecificOptions>,
395+
locals: string[],
396+
scope: ScopeLocals
397+
) {
398+
function add(name: string) {
399+
if (locals.includes(name)) return;
400+
locals.push(name);
401+
}
402+
403+
let astScope: string[] = [];
404+
function isInScope(name: string) {
405+
return astScope.flat().includes(name);
406+
}
407+
408+
function isInJsScope(name: string) {
409+
if (jsPath.scope.getBinding(name)) return true;
410+
if (['this', 'globalThis'].includes(name)) return true;
411+
if (scope.locals.includes(name)) return true;
412+
if (state.originalImportedNames.has(name)) {
413+
return true;
399414
}
415+
return false;
400416
}
401-
return scopeLocals;
417+
418+
return (_env: ASTPluginEnvironment): { name: string; visitor: NodeVisitor } => {
419+
return {
420+
name: 'discover-locals',
421+
visitor: {
422+
All: {
423+
enter(_node, path) {
424+
const blockParams = (path.parentNode as any)?.blockParams;
425+
if (blockParams && ['children', 'body'].includes(path.parentKey!)) {
426+
astScope.push(blockParams);
427+
}
428+
},
429+
exit(_node, path) {
430+
const blockParams = (path.parentNode as any)?.blockParams;
431+
if (blockParams && ['children', 'body'].includes(path.parentKey!)) {
432+
const i = astScope.indexOf(blockParams);
433+
astScope.splice(i, 1);
434+
}
435+
},
436+
},
437+
PathExpression(node) {
438+
if (
439+
node.head.type === 'VarHead' &&
440+
!isInScope(node.head.name) &&
441+
isInJsScope(node.head.name)
442+
) {
443+
add(node.head.name);
444+
}
445+
},
446+
ElementNode(node) {
447+
const name = node.tag.split('.')[0];
448+
if (!isInScope(name) && isInJsScope(name)) {
449+
add(name);
450+
}
451+
},
452+
},
453+
};
454+
};
402455
}
403456

404457
function buildPrecompileOptions<EnvSpecificOptions>(
@@ -437,6 +490,9 @@ function buildPrecompileOptions<EnvSpecificOptions>(
437490
},
438491
};
439492

493+
const locals: string[] = [];
494+
output.plugins?.ast?.push(discoverLocals(target, state, locals, scope));
495+
440496
for (let [key, value] of Object.entries(userTypedOptions)) {
441497
if (key !== 'scope') {
442498
// `scope` in the user-facing API becomes `locals` in the low-level
@@ -445,7 +501,7 @@ function buildPrecompileOptions<EnvSpecificOptions>(
445501
}
446502
}
447503

448-
output.locals = scope.locals;
504+
output.locals = locals;
449505

450506
if (config.rfc931Support) {
451507
output.strictMode = true;
@@ -483,7 +539,7 @@ function insertCompiledTemplate<EnvSpecificOptions>(
483539
backingClass: NodePath<Parameters<typeof t.callExpression>[1][number]> | undefined
484540
) {
485541
let t = babel.types;
486-
let scopeLocals = buildScopeLocals(userTypedOptions, config, template);
542+
let scopeLocals = buildScopeLocals(userTypedOptions, config);
487543
let options = buildPrecompileOptions(
488544
babel,
489545
target,
@@ -513,6 +569,8 @@ function insertCompiledTemplate<EnvSpecificOptions>(
513569
configFile: false,
514570
}) as t.File;
515571

572+
scopeLocals.locals.length = 0;
573+
scopeLocals.locals.push(...options.locals!);
516574
ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
517575
remapIdentifiers(precompileResultAST, babel, scopeLocals);
518576

@@ -560,7 +618,7 @@ function insertTransformedTemplate<EnvSpecificOptions>(
560618
backingClass: NodePath<Parameters<typeof t.callExpression>[1][number]> | undefined
561619
) {
562620
let t = babel.types;
563-
let scopeLocals = buildScopeLocals(userTypedOptions, formatOptions, template);
621+
let scopeLocals = buildScopeLocals(userTypedOptions, formatOptions);
564622
let options = buildPrecompileOptions(
565623
babel,
566624
target,
@@ -574,14 +632,14 @@ function insertTransformedTemplate<EnvSpecificOptions>(
574632
let transformed = print(ast, { entityEncoding: 'raw' });
575633
if (target.isCallExpression()) {
576634
(target.get('arguments.0') as NodePath<t.Node>).replaceWith(t.stringLiteral(transformed));
577-
if (!scopeLocals.isEmpty()) {
578-
if (!formatOptions.enableScope) {
579-
maybePruneImport(state.util, target.get('callee'));
580-
target.set('callee', precompileTemplate(state.util, target));
581-
}
582-
ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
583-
updateScope(babel, target, scopeLocals);
635+
if (!formatOptions.enableScope) {
636+
maybePruneImport(state.util, target.get('callee'));
637+
target.set('callee', precompileTemplate(state.util, target));
584638
}
639+
scopeLocals.locals.length = 0;
640+
scopeLocals.locals.push(...options.locals!);
641+
ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
642+
updateScope(babel, target, scopeLocals);
585643

586644
if (formatOptions.rfc931Support === 'polyfilled') {
587645
maybePruneImport(state.util, target.get('callee'));
@@ -615,6 +673,8 @@ function insertTransformedTemplate<EnvSpecificOptions>(
615673
t.callExpression(precompileTemplate(state.util, target), [t.stringLiteral(transformed)])
616674
)[0];
617675
ensureImportedNames(newCall, scopeLocals, state.util, state.originalImportedNames);
676+
scopeLocals.locals.length = 0;
677+
scopeLocals.locals.push(...options.locals!);
618678
updateScope(babel, newCall, scopeLocals);
619679
} else {
620680
(target.get('quasi').get('quasis.0') as NodePath<t.TemplateElement>).replaceWith(
@@ -635,14 +695,13 @@ function templateFactoryConfig(opts: NormalizedOpts) {
635695

636696
function buildScope(babel: typeof Babel, locals: ScopeLocals) {
637697
let t = babel.types;
698+
638699
return t.arrowFunctionExpression(
639700
[],
640701
t.objectExpression(
641-
locals
642-
.entries()
643-
.map(([name, identifier]) =>
644-
t.objectProperty(t.identifier(name), t.identifier(identifier), false, true)
645-
)
702+
locals.entries().map(([name, identifier]) =>
703+
t.objectProperty(t.identifier(name), t.identifier(identifier), false, true)
704+
)
646705
)
647706
);
648707
}
@@ -656,13 +715,16 @@ function updateScope(babel: typeof Babel, target: NodePath<t.CallExpression>, lo
656715
});
657716
if (scope) {
658717
scope.set('value', buildScope(babel, locals));
659-
} else {
718+
if (locals.isEmpty()) {
719+
scope.remove();
720+
}
721+
} else if (!locals.isEmpty()) {
660722
secondArg.pushContainer(
661723
'properties',
662724
t.objectProperty(t.identifier('scope'), buildScope(babel, locals))
663725
);
664726
}
665-
} else {
727+
} else if (!locals.isEmpty()) {
666728
target.pushContainer(
667729
'arguments',
668730
t.objectExpression([t.objectProperty(t.identifier('scope'), buildScope(babel, locals))])

‎src/scope-locals.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,15 @@ export class ScopeLocals {
3131
}
3232

3333
entries() {
34-
return Object.entries(this.#mapping);
34+
let mapping: Record<string, string> = {};
35+
this.locals.forEach((name) => {
36+
mapping[name] = this.mapping[name] || name;
37+
});
38+
return Object.entries(mapping);
39+
}
40+
41+
get mapping() {
42+
return this.#mapping;
3543
}
3644

3745
add(key: string, value?: string) {

‎test/transform.test.ts

+21-13
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,7 @@ describe('htmlbars-inline-precompile', function () {
11151115
const template = precompileTemplate("<Message @text={{two}} />", {
11161116
moduleName: 'customModuleName',
11171117
scope: () => ({
1118+
Message,
11181119
two
11191120
})
11201121
});"
@@ -1537,7 +1538,7 @@ describe('htmlbars-inline-precompile', function () {
15371538

15381539
describe('scope', function () {
15391540
it('correctly handles scope function (non-block arrow function)', function () {
1540-
let source = 'hello';
1541+
let source = '<foo /><bar/>';
15411542
let spy = sinon.spy(compiler, 'precompile');
15421543

15431544
transform(
@@ -1547,7 +1548,7 @@ describe('htmlbars-inline-precompile', function () {
15471548
});
15481549

15491550
it('correctly handles scope function (block arrow function)', function () {
1550-
let source = 'hello';
1551+
let source = '<foo /><bar/>';
15511552
let spy = sinon.spy(compiler, 'precompile');
15521553

15531554
transform(
@@ -1558,7 +1559,7 @@ describe('htmlbars-inline-precompile', function () {
15581559
});
15591560

15601561
it('correctly handles scope function (normal function)', function () {
1561-
let source = 'hello';
1562+
let source = '<foo /><bar/>';
15621563
let spy = sinon.spy(compiler, 'precompile');
15631564

15641565
transform(
@@ -1569,7 +1570,7 @@ describe('htmlbars-inline-precompile', function () {
15691570
});
15701571

15711572
it('correctly handles scope function (object method)', function () {
1572-
let source = 'hello';
1573+
let source = '<foo /><bar/>';
15731574
let spy = sinon.spy(compiler, 'precompile');
15741575

15751576
transform(
@@ -1579,7 +1580,7 @@ describe('htmlbars-inline-precompile', function () {
15791580
});
15801581

15811582
it('correctly handles scope function with coverage', function () {
1582-
let source = 'hello';
1583+
let source = '<foo /><bar/>';
15831584
let spy = sinon.spy(compiler, 'precompile');
15841585

15851586
transform(
@@ -1674,7 +1675,7 @@ describe('htmlbars-inline-precompile', function () {
16741675
// whether you've (for example) capitalized your variable identifier.
16751676
//
16761677
// needs https://github.com/glimmerjs/glimmer-vm/pull/1421
1677-
it.skip('shadows html elements with locals', function () {
1678+
it('shadows html elements with locals', function () {
16781679
plugins = [
16791680
[
16801681
HTMLBarsInlinePrecompile,
@@ -1693,11 +1694,16 @@ describe('htmlbars-inline-precompile', function () {
16931694
);
16941695

16951696
expect(transformed).toMatchInlineSnapshot(`
1696-
import templateOnly from "@ember/component/template-only";
1697+
"import { precompileTemplate } from "@ember/template-compilation";
16971698
import { setComponentTemplate } from "@ember/component";
1698-
import { precompileTemplate } from "@ember/template-compilation";
1699+
import templateOnly from "@ember/component/template-only";
16991700
let div = 1;
1700-
export default setComponentTemplate(precompileTemplate('<div></div>', { scope: () => ({ div }), strictMode: true }), templateOnly());
1701+
export default setComponentTemplate(precompileTemplate("<div></div>", {
1702+
scope: () => ({
1703+
div
1704+
}),
1705+
strictMode: true
1706+
}), templateOnly());"
17011707
`);
17021708
});
17031709

@@ -1734,7 +1740,7 @@ describe('htmlbars-inline-precompile', function () {
17341740
});
17351741

17361742
// needs https://github.com/glimmerjs/glimmer-vm/pull/1421
1737-
it.skip('leaves ember keywords alone when no local is defined', function () {
1743+
it('leaves ember keywords alone when no local is defined', function () {
17381744
plugins = [
17391745
[
17401746
HTMLBarsInlinePrecompile,
@@ -1752,10 +1758,12 @@ describe('htmlbars-inline-precompile', function () {
17521758
);
17531759

17541760
expect(transformed).toMatchInlineSnapshot(`
1755-
import templateOnly from "@ember/component/template-only";
1761+
"import { precompileTemplate } from "@ember/template-compilation";
17561762
import { setComponentTemplate } from "@ember/component";
1757-
import { precompileTemplate } from "@ember/template-compilation";
1758-
export default setComponentTemplate(precompileTemplate('{{hasBlock "thing"}}', { strictMode: true }), templateOnly());
1763+
import templateOnly from "@ember/component/template-only";
1764+
export default setComponentTemplate(precompileTemplate("{{hasBlock \\"thing\\"}}", {
1765+
strictMode: true
1766+
}), templateOnly());"
17591767
`);
17601768
});
17611769

0 commit comments

Comments
 (0)