From 7fe1c21c60c2196ddcd4504e57d735ac5f1845d3 Mon Sep 17 00:00:00 2001 From: void_malex Date: Sat, 17 Feb 2024 17:04:26 +0000 Subject: [PATCH 01/19] resolver transform to emit imports for helper and modifiers that need them in strict mode templates --- packages/compat/src/resolver-transform.ts | 71 +++++++++++++++++------ 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 7c469c33b..99ee0c333 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -85,6 +85,15 @@ export const builtInKeywords = [ 'yield', ]; +export const builtInHelperKeywordsWithImports = ['array', 'concat', 'fn', 'get', 'hash', 'unique-id']; +export const builtInModifierKeywordsWithImports = ['on']; + +const STRING_CAMELIZE_REGEXP_1 = /(\-|\_|\.|\s)+(.)?/g; + +const camelize = function (key: string) { + return key.replace(STRING_CAMELIZE_REGEXP_1, (_match, _separator, chr) => (chr ? chr.toUpperCase() : '')); +}; + interface ComponentResolution { type: 'component'; specifier: string; @@ -92,18 +101,21 @@ interface ComponentResolution { yieldsArguments: Required['yieldsArguments']; argumentsAreComponents: string[]; nameHint: string; + namedImport?: string; } type HelperResolution = { type: 'helper'; - nameHint: string; + nameHint?: string; + namedImport?: string; specifier: string; }; type ModifierResolution = { type: 'modifier'; specifier: string; - nameHint: string; + nameHint?: string; + namedImport?: string; }; type ResolutionResult = ComponentResolution | HelperResolution | ModifierResolution; @@ -157,9 +169,14 @@ class TemplateResolver implements ASTPlugin { case 'component': case 'modifier': case 'helper': { - let name = this.env.meta.jsutils.bindImport(resolution.specifier, 'default', parentPath, { - nameHint: resolution.nameHint, - }); + let name = this.env.meta.jsutils.bindImport( + resolution.specifier, + resolution.namedImport ?? 'default', + parentPath, + { + nameHint: resolution.nameHint, + } + ); setter(parentPath.node, this.env.syntax.builders.path(name)); return; } @@ -437,15 +454,23 @@ class TemplateResolver implements ASTPlugin { // globally-named helpers. It throws an error. So it's fine for us to // prioritize the builtIns here without bothering to resolve a user helper // of the same name. - if (builtInKeywords.includes(path)) { + const importedBuiltIn = exports.builtInHelperKeywordsWithImports.includes(path); + if (builtInKeywords.includes(path) && !importedBuiltIn) { return null; } - - return { - type: 'helper', - specifier: `#embroider_compat/helpers/${path}`, - nameHint: this.nameHint(path), - }; + if (importedBuiltIn) { + return { + type: 'helper', + specifier: `@ember/helper`, + namedImport: camelize(path), + }; + } else { + return { + type: 'helper', + specifier: `#embroider_compat/helpers/${path}`, + nameHint: this.nameHint(path), + }; + } } private targetHelperOrComponent( @@ -571,15 +596,25 @@ class TemplateResolver implements ASTPlugin { if (!this.staticModifiersEnabled) { return null; } - if (builtInKeywords.includes(path)) { + + const importedBuiltIn = builtInModifierKeywordsWithImports.includes(path); + if (builtInKeywords.includes(path) && !importedBuiltIn) { return null; } - return { - type: 'modifier', - specifier: `#embroider_compat/modifiers/${path}`, - nameHint: this.nameHint(path), - }; + if (importedBuiltIn) { + return { + type: 'modifier', + specifier: `@ember/modifier`, + namedImport: path, + }; + } else { + return { + type: 'modifier', + specifier: `#embroider_compat/modifiers/${path}`, + nameHint: this.nameHint(path), + }; + } } targetDynamicModifier(modifier: ComponentLocator, loc: Loc): ModifierResolution | ResolutionFail | null { From d1fc68525ba575b792d2ab6843d5a8012d23b0ae Mon Sep 17 00:00:00 2001 From: void_malex Date: Sat, 17 Feb 2024 20:43:35 +0000 Subject: [PATCH 02/19] added built in components test updates to match new expectations --- packages/compat/src/resolver-transform.ts | 49 ++++++++++++++--------- tests/scenarios/compat-resolver-test.ts | 31 ++++++++++---- 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 99ee0c333..818badc55 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -85,22 +85,17 @@ export const builtInKeywords = [ 'yield', ]; +export const builtInComponentKeywordsWithImports = ['input', 'textarea', 'link-to']; export const builtInHelperKeywordsWithImports = ['array', 'concat', 'fn', 'get', 'hash', 'unique-id']; export const builtInModifierKeywordsWithImports = ['on']; -const STRING_CAMELIZE_REGEXP_1 = /(\-|\_|\.|\s)+(.)?/g; - -const camelize = function (key: string) { - return key.replace(STRING_CAMELIZE_REGEXP_1, (_match, _separator, chr) => (chr ? chr.toUpperCase() : '')); -}; - interface ComponentResolution { type: 'component'; specifier: string; yieldsComponents: Required['yieldsSafeComponents']; yieldsArguments: Required['yieldsArguments']; argumentsAreComponents: string[]; - nameHint: string; + nameHint?: string; namedImport?: string; } @@ -387,22 +382,34 @@ class TemplateResolver implements ASTPlugin { return null; } - if (builtInKeywords.includes(name)) { + const importedBuiltIn = builtInComponentKeywordsWithImports.includes(name); + if (builtInKeywords.includes(name) && !importedBuiltIn) { return null; } if (this.isIgnoredComponent(name)) { return null; } - let componentRules = this.rules.components.get(name); - return { - type: 'component', - specifier: `#embroider_compat/components/${name}`, - yieldsComponents: componentRules ? componentRules.yieldsSafeComponents : [], - yieldsArguments: componentRules ? componentRules.yieldsArguments : [], - argumentsAreComponents: componentRules ? componentRules.argumentsAreComponents : [], - nameHint: this.nameHint(name), - }; + if (importedBuiltIn) { + return { + type: 'component', + specifier: name === 'link-to' ? '@ember/routing' : '@ember/component', + yieldsComponents: [], + yieldsArguments: [], + argumentsAreComponents: [], + namedImport: capitalize(camelize(name)), + }; + } else { + let componentRules = this.rules.components.get(name); + return { + type: 'component', + specifier: `#embroider_compat/components/${name}`, + yieldsComponents: componentRules ? componentRules.yieldsSafeComponents : [], + yieldsArguments: componentRules ? componentRules.yieldsArguments : [], + argumentsAreComponents: componentRules ? componentRules.argumentsAreComponents : [], + nameHint: this.nameHint(name), + }; + } } private targetComponentHelper( @@ -454,7 +461,7 @@ class TemplateResolver implements ASTPlugin { // globally-named helpers. It throws an error. So it's fine for us to // prioritize the builtIns here without bothering to resolve a user helper // of the same name. - const importedBuiltIn = exports.builtInHelperKeywordsWithImports.includes(path); + const importedBuiltIn = builtInHelperKeywordsWithImports.includes(path); if (builtInKeywords.includes(path) && !importedBuiltIn) { return null; } @@ -1065,6 +1072,12 @@ function capitalize(word: string): string { return word[0].toUpperCase() + word.slice(1); } +const STRING_CAMELIZE_REGEXP_1 = /(\-|\_|\.|\s)+(.)?/g; + +function camelize(key: string) { + return key.replace(STRING_CAMELIZE_REGEXP_1, (_match, _separator, chr) => (chr ? chr.toUpperCase() : '')); +} + // ElementNodes have both children and attributes and both of those are // "children" in the abstract syntax tree sense, but here we want to distinguish // between them. diff --git a/tests/scenarios/compat-resolver-test.ts b/tests/scenarios/compat-resolver-test.ts index d3ed4df4c..18ee40710 100644 --- a/tests/scenarios/compat-resolver-test.ts +++ b/tests/scenarios/compat-resolver-test.ts @@ -1062,28 +1062,41 @@ Scenarios.fromProject(() => new Project()) `); }); - test('built-in components are ignored when used with the component helper', async function () { + test('built-in components are imported when used with the component helper', async function () { givenFiles({ 'templates/application.hbs': `{{component "input"}}{{component "link-to"}}{{component "textarea"}}`, }); await configure({ staticComponents: true }); expectTranspiled('templates/application.hbs').equalsCode(` import { precompileTemplate } from "@ember/template-compilation"; - export default precompileTemplate("{{component \\"input\\"}}{{component \\"link-to\\"}}{{component \\"textarea\\"}}", { - moduleName: "my-app/templates/application.hbs" + import { Input, Textarea } from "@ember/component"; + import { LinkTo } from "@ember/routing"; + export default precompileTemplate("{{component Input}}{{component LinkTo}}{{component Textarea}}", { + moduleName: "my-app/templates/application.hbs", + scope: () => ({ + Input, + LinkTo, + Textarea + }), }); `); }); - test('built-in helpers are ignored when used with the helper keyword', async function () { + test('built-in helpers are imported when used with the helper keyword', async function () { givenFiles({ 'templates/application.hbs': `{{helper "fn"}}{{helper "array"}}{{helper "concat"}}`, }); await configure({ staticHelpers: true }); expectTranspiled('templates/application.hbs').equalsCode(` import { precompileTemplate } from "@ember/template-compilation"; - export default precompileTemplate("{{helper \\"fn\\"}}{{helper \\"array\\"}}{{helper \\"concat\\"}}", { - moduleName: "my-app/templates/application.hbs" + import { fn, array, concat } from "@ember/helper"; + export default precompileTemplate("{{helper fn}}{{helper array}}{{helper concat}}", { + moduleName: "my-app/templates/application.hbs", + scope: () => ({ + fn, + array, + concat + }), }); `); }); @@ -1115,8 +1128,12 @@ Scenarios.fromProject(() => new Project()) await configure({ staticModifiers: true }); expectTranspiled('templates/application.hbs').equalsCode(` import { precompileTemplate } from "@ember/template-compilation"; + import { on } from "@ember/modifier"; export default precompileTemplate("\\n {{outlet}}\\n {{yield bar}}\\n {{#with (hash submit=(action doit)) as |thing|}}\\n {{/with}}\\n \\n
\\n ", { - moduleName: "my-app/templates/application.hbs" + moduleName: "my-app/templates/application.hbs", + scope: () => ({ + on, + }), }); `); }); From 87b8604bd7267a031093da474547cc7018f89b14 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 20 Feb 2024 16:42:37 -0500 Subject: [PATCH 03/19] adding coverage for direct invocation of built-ins --- tests/scenarios/compat-resolver-test.ts | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/scenarios/compat-resolver-test.ts b/tests/scenarios/compat-resolver-test.ts index 18ee40710..c4379b678 100644 --- a/tests/scenarios/compat-resolver-test.ts +++ b/tests/scenarios/compat-resolver-test.ts @@ -1082,6 +1082,26 @@ Scenarios.fromProject(() => new Project()) `); }); + test('built-in components are imported when used directly', async function () { + givenFiles({ + 'templates/application.hbs': `