From b3e51f6b1d261da09918f4899f76a5d8e5f6a440 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 7 May 2024 12:47:31 -0400 Subject: [PATCH 1/5] Failing test --- __tests__/tests.ts | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index a6dcfaa..ac142cb 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -2079,6 +2079,64 @@ describe('htmlbars-inline-precompile', function () { `); }); + it('works with two components', function () { + plugins = [ + [ + HTMLBarsInlinePrecompile, + { + targetFormat: 'hbs', + }, + ], + ]; + + let p = new Preprocessor(); + + let transformed = transform( + p.process( + ` + import Component from '@glimmer/component'; + + export default class Test extends Component { + foo = 1; + + + } + + const Icon = ; + ` + ) + ); + + expect(transformed).toEqualCode(` + import Component from "@glimmer/component"; + import { precompileTemplate } from "@ember/template-compilation"; + import { setComponentTemplate } from "@ember/component"; + import templateOnly from "@ember/component/template-only"; + export default class Test extends Component { + foo = 1; + static { + setComponentTemplate( + precompileTemplate("\\n \\n ", { + strictMode: true, + scope: () => ({ + Icon, + }), + }), + this + ); + } + } + const Icon = setComponentTemplate( + precompileTemplate("Icon", { + strictMode: true, + }), + templateOnly() + ); + `); + }); + it('works for class member form with `this` references', function () { plugins = [ [ From d1bb91f35a2a5075d784f8700df3fa2cc1d6698f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 7 May 2024 15:43:06 -0400 Subject: [PATCH 2/5] Add another failing test --- __tests__/tests.ts | 89 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index ac142cb..c0f86c4 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -1478,6 +1478,66 @@ describe('htmlbars-inline-precompile', function () { `); }); + it('cleans up leftover imports when polyfilling rfc931 with hbs target', function () { + plugins = [ + [ + HTMLBarsInlinePrecompile, + { + targetFormat: 'hbs', + }, + ], + ]; + let code = ` + import { template } from "@ember/template-compiler"; + import Component from '@glimmer/component'; + export default class Test extends Component { + foo = 1; + static{ + template("", { + component: this, + eval () { + return eval(arguments[0]); + } + }); + } + } + const Icon = template("Icon", { + eval () { + return eval(arguments[0]); + } + }); + `; + + let transformed = transform(code); + + expect(transformed).toEqualCode(` + import Component from "@glimmer/component"; + import { precompileTemplate } from "@ember/template-compilation"; + import { setComponentTemplate } from "@ember/component"; + import templateOnly from "@ember/component/template-only"; + export default class Test extends Component { + foo = 1; + static { + setComponentTemplate( + precompileTemplate("", { + strictMode: true, + scope: () => ({ + Icon, + }), + }), + this + ); + } + } + const Icon = setComponentTemplate( + precompileTemplate("Icon", { + strictMode: true, + }), + templateOnly() + ); + `); + }); + it("respects user's strict option on template()", function () { plugins = [ [ @@ -2090,25 +2150,24 @@ describe('htmlbars-inline-precompile', function () { ]; let p = new Preprocessor(); + let processed = p.process( + ` + import Component from '@glimmer/component'; - let transformed = transform( - p.process( - ` - import Component from '@glimmer/component'; - - export default class Test extends Component { - foo = 1; + export default class Test extends Component { + foo = 1; - - } + + } - const Icon = ; - ` - ) + const Icon = ; + ` ); + let transformed = transform(processed); + expect(transformed).toEqualCode(` import Component from "@glimmer/component"; import { precompileTemplate } from "@ember/template-compilation"; @@ -2118,7 +2177,7 @@ describe('htmlbars-inline-precompile', function () { foo = 1; static { setComponentTemplate( - precompileTemplate("\\n \\n ", { + precompileTemplate("\\n \\n ", { strictMode: true, scope: () => ({ Icon, From 8d955098c258810d5e7416fed1884bdb70aa5ddc Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 7 May 2024 15:44:02 -0400 Subject: [PATCH 3/5] original failing test not needed, new test is more minimal --- __tests__/tests.ts | 57 ---------------------------------------------- 1 file changed, 57 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index c0f86c4..2aeb8bf 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -2139,63 +2139,6 @@ describe('htmlbars-inline-precompile', function () { `); }); - it('works with two components', function () { - plugins = [ - [ - HTMLBarsInlinePrecompile, - { - targetFormat: 'hbs', - }, - ], - ]; - - let p = new Preprocessor(); - let processed = p.process( - ` - import Component from '@glimmer/component'; - - export default class Test extends Component { - foo = 1; - - - } - - const Icon = ; - ` - ); - - let transformed = transform(processed); - - expect(transformed).toEqualCode(` - import Component from "@glimmer/component"; - import { precompileTemplate } from "@ember/template-compilation"; - import { setComponentTemplate } from "@ember/component"; - import templateOnly from "@ember/component/template-only"; - export default class Test extends Component { - foo = 1; - static { - setComponentTemplate( - precompileTemplate("\\n \\n ", { - strictMode: true, - scope: () => ({ - Icon, - }), - }), - this - ); - } - } - const Icon = setComponentTemplate( - precompileTemplate("Icon", { - strictMode: true, - }), - templateOnly() - ); - `); - }); - it('works for class member form with `this` references', function () { plugins = [ [ From f180ad9a88849382e64a60647294ef25b771b2da Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 7 May 2024 18:00:32 -0400 Subject: [PATCH 4/5] Fix --- src/plugin.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/plugin.ts b/src/plugin.ts index 3bfd9d4..62ea3d5 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -629,6 +629,7 @@ function updateCallForm( ), ]) ); + // we just wrapped the target callExpression in the call to // setComponentTemplate. Adjust `target` back to point at the // precompileTemplate call for the final updateScope below. @@ -636,6 +637,15 @@ function updateCallForm( target = target.get('arguments.0') as NodePath; } + // Changing imports/identifiers doesn't automatically update the scope. + // + // NOTE: https://github.com/babel/babel/issues/7974 + // + // We need to refresh ourselves + // This is expensive, but required so that maybePruneImport + // has a chance to prune imports when their specifiers are no longer used. + state.program.scope.crawl(); + // We deliberately do updateScope at the end so that when it updates // references, those references will point to the accurate paths in the // final AST. From 587cc9013f55d39c588c002e4b9e568fe7f7a631 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 7 May 2024 22:02:01 -0400 Subject: [PATCH 5/5] adjust references when pruning away callsites --- __tests__/tests.ts | 2 +- src/plugin.ts | 31 ++++++++++++++----------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index 2aeb8bf..774bf03 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -1478,7 +1478,7 @@ describe('htmlbars-inline-precompile', function () { `); }); - it('cleans up leftover imports when polyfilling rfc931 with hbs target', function () { + it('cleans up leftover imports when there is more than one template', function () { plugins = [ [ HTMLBarsInlinePrecompile, diff --git a/src/plugin.ts b/src/plugin.ts index 62ea3d5..cbe4e03 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -629,23 +629,12 @@ function updateCallForm( ), ]) ); - // we just wrapped the target callExpression in the call to // setComponentTemplate. Adjust `target` back to point at the // precompileTemplate call for the final updateScope below. // target = target.get('arguments.0') as NodePath; } - - // Changing imports/identifiers doesn't automatically update the scope. - // - // NOTE: https://github.com/babel/babel/issues/7974 - // - // We need to refresh ourselves - // This is expensive, but required so that maybePruneImport - // has a chance to prune imports when their specifiers are no longer used. - state.program.scope.crawl(); - // We deliberately do updateScope at the end so that when it updates // references, those references will point to the accurate paths in the // final AST. @@ -774,18 +763,26 @@ function maybePruneImport( return; } let binding = identifier.scope.getBinding(identifier.node.name); - // this checks if the identifier (that we're about to remove) is used in - // exactly one place. - if ( - binding?.referencePaths.reduce((count, path) => (path.removed ? count : count + 1), 0) === 1 - ) { + + if (!binding) { + return; + } + + let found = binding.referencePaths.find((path) => path.node === identifier.node); + if (!found) { + return; + } + + binding.referencePaths.splice(binding.referencePaths.indexOf(found), 1); + binding.references--; + + if (binding.references === 0) { let specifier = binding.path; if (specifier.isImportSpecifier()) { let declaration = specifier.parentPath as NodePath; util.removeImport(declaration.node.source.value, name(specifier.node.imported)); } } - identifier.removed = true; } function precompileTemplate(i: Importer) {