diff --git a/packages/compat/src/audit.ts b/packages/compat/src/audit.ts index dac05d2ff..2a7a829f5 100644 --- a/packages/compat/src/audit.ts +++ b/packages/compat/src/audit.ts @@ -67,12 +67,11 @@ interface InternalModule { isCJS: boolean; isAMD: boolean; dependencies: string[]; + transpiledContent: string | Buffer; }; resolved?: Map; - content?: string | Buffer; - linked?: { exports: Set; }; @@ -140,7 +139,7 @@ export class AuditResults { })) : [], exports: module.linked?.exports ? [...module.linked.exports] : [], - content: module.content ? module.content.toString() : '', + content: module.parsed?.transpiledContent ? module.parsed?.transpiledContent.toString() : '', }; results.modules[explicitRelative(baseDir, filename)] = publicModule; } @@ -332,7 +331,6 @@ export class Audit { } else { module.parsed = visitResult; module.resolved = await this.resolveDeps(visitResult.dependencies, filename); - module.content = content; } } } @@ -479,6 +477,7 @@ export class Audit { isCJS: false, isAMD: false, dependencies, + transpiledContent: content, }; } @@ -504,6 +503,7 @@ export class Audit { isCJS: result.isCJS, isAMD: result.isAMD, dependencies: result.imports.map(i => i.source), + transpiledContent: result.transpiledContent, }; } catch (err) { if (['BABEL_PARSE_ERROR', 'BABEL_TRANSFORM_ERROR'].includes(err.code)) { diff --git a/packages/compat/src/audit/babel-visitor.ts b/packages/compat/src/audit/babel-visitor.ts index 2cf57cc1a..fe8eff462 100644 --- a/packages/compat/src/audit/babel-visitor.ts +++ b/packages/compat/src/audit/babel-visitor.ts @@ -41,10 +41,10 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf let sawDefine: boolean = false; /* eslint-enable @typescript-eslint/no-inferrable-types */ - let ast = transformSync(rawSource, Object.assign({ filename: filename }, babelConfig))!.ast!; + let { ast, code } = transformSync(rawSource, Object.assign({ filename: filename }, babelConfig))!; let saveCodeFrame = frames.forSource(rawSource); - traverse(ast, { + traverse(ast!, { Identifier(path: NodePath) { if (path.node.name === 'module' && isFreeVariable(path)) { sawModule = true; @@ -154,7 +154,7 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf let isCJS = imports.length === 0 && exports.size === 0 && (sawModule || sawExports); let isAMD = imports.length === 0 && exports.size === 0 && sawDefine; - return { imports, exports, isCJS, isAMD, problems }; + return { imports, exports, isCJS, isAMD, problems, transpiledContent: code! }; } export class CodeFrameStorage { diff --git a/packages/compat/src/babel-plugin-adjust-imports.ts b/packages/compat/src/babel-plugin-adjust-imports.ts index bb761a7e8..20f2f849d 100644 --- a/packages/compat/src/babel-plugin-adjust-imports.ts +++ b/packages/compat/src/babel-plugin-adjust-imports.ts @@ -5,9 +5,9 @@ import type { types as t } from '@babel/core'; import { ImportUtil } from 'babel-import-util'; import { readJSONSync } from 'fs-extra'; import { CompatResolverOptions } from './resolver-transform'; -import { Resolver } from '@embroider/core'; +import { Package, packageName, Resolver, unrelativize } from '@embroider/core'; import { snippetToDasherizedName } from './dasherize-component-name'; -import { ModuleRules, TemplateRules } from './dependency-rules'; +import { ActivePackageRules, ComponentRules, ModuleRules, TemplateRules } from './dependency-rules'; export type Options = { appRoot: string }; @@ -16,15 +16,23 @@ interface State { } type BabelTypes = typeof t; + +interface ExtraImports { + [key: string]: { + dependsOnComponents?: string[]; // these are already standardized in dasherized form + dependsOnModules?: string[]; + }; +} + type InternalConfig = { resolverOptions: CompatResolverOptions; resolver: Resolver; - extraImports: { - [absPath: string]: { - dependsOnComponents?: string[]; // these are already standardized in dasherized form - dependsOnModules?: string[]; - }; - }; + + // rule-based extra dependencies, indexed by filename + extraImports: ExtraImports; + + // rule-based extra dependencies, indexed by classical component name + componentExtraImports: ExtraImports; }; export default function main(babel: typeof Babel) { @@ -39,6 +47,7 @@ export default function main(babel: typeof Babel) { resolverOptions, resolver: new Resolver(resolverOptions), extraImports: preprocessExtraImports(resolverOptions), + componentExtraImports: preprocessComponentExtraImports(resolverOptions), }; return cached; } @@ -61,35 +70,57 @@ export default function main(babel: typeof Babel) { function addExtraImports(t: BabelTypes, path: NodePath, config: InternalConfig) { let filename: string = path.hub.file.opts.filename; let entry = config.extraImports[filename]; + let adder = new ImportUtil(t, path); if (entry) { - let adder = new ImportUtil(t, path); - if (entry.dependsOnModules) { - for (let target of entry.dependsOnModules) { - path.node.body.unshift(amdDefine(t, adder, path, target, target)); - } + applyRules(t, path, entry, adder, config, filename); + } + + let componentName = config.resolver.reverseComponentLookup(filename); + if (componentName) { + let rules = config.componentExtraImports[componentName]; + if (rules) { + applyRules(t, path, rules, adder, config, filename); } - if (entry.dependsOnComponents) { - for (let dasherizedName of entry.dependsOnComponents) { - let pkg = config.resolver.owningPackage(filename); - if (pkg) { - let owningEngine = config.resolver.owningEngine(pkg); - if (owningEngine) { - path.node.body.unshift( - amdDefine( - t, - adder, - path, - `#embroider_compat/components/${dasherizedName}`, - `${owningEngine.packageName}/components/${dasherizedName}` - ) - ); - } + } +} + +function applyRules( + t: BabelTypes, + path: NodePath, + rules: ExtraImports[string], + adder: ImportUtil, + config: InternalConfig, + filename: string +) { + let lookup = lazyPackageLookup(config, filename); + if (rules.dependsOnModules) { + for (let target of rules.dependsOnModules) { + if (lookup.owningPackage) { + let runtimeName: string; + if (packageName(target)) { + runtimeName = target; + } else { + runtimeName = unrelativize(lookup.owningPackage, target, filename); } + path.node.body.unshift(amdDefine(t, adder, path, target, runtimeName)); + } + } + } + if (rules.dependsOnComponents) { + for (let dasherizedName of rules.dependsOnComponents) { + if (lookup.owningEngine) { + path.node.body.unshift( + amdDefine( + t, + adder, + path, + `#embroider_compat/components/${dasherizedName}`, + `${lookup.owningEngine.packageName}/components/${dasherizedName}` + ) + ); } } } - - //let componentName = config.resolver.reverseComponentLookup(filename); } function amdDefine(t: BabelTypes, adder: ImportUtil, path: NodePath, target: string, runtimeName: string) { @@ -102,8 +133,8 @@ function amdDefine(t: BabelTypes, adder: ImportUtil, path: NodePath, ); } -function preprocessExtraImports(config: CompatResolverOptions): InternalConfig['extraImports'] { - let extraImports: InternalConfig['extraImports'] = {}; +function preprocessExtraImports(config: CompatResolverOptions): ExtraImports { + let extraImports: ExtraImports = {}; for (let rule of config.activePackageRules) { if (rule.addonModules) { for (let [filename, moduleRules] of Object.entries(rule.addonModules)) { @@ -133,6 +164,60 @@ function preprocessExtraImports(config: CompatResolverOptions): InternalConfig[' return extraImports; } +function lazyPackageLookup(config: InternalConfig, filename: string) { + let owningPackage: { result: Package | undefined } | undefined; + let owningEngine: { result: ReturnType | undefined } | undefined; + return { + get owningPackage() { + if (!owningPackage) { + owningPackage = { result: config.resolver.owningPackage(filename) }; + } + return owningPackage.result; + }, + get owningEngine() { + if (!owningEngine) { + owningEngine = { result: undefined }; + let p = this.owningPackage; + if (p) { + owningEngine.result = config.resolver.owningEngine(p); + } + } + return owningEngine.result; + }, + }; +} + +function preprocessComponentExtraImports(config: CompatResolverOptions): ExtraImports { + let extraImports: ExtraImports = {}; + for (let rule of config.activePackageRules) { + if (rule.components) { + for (let [componentName, rules] of Object.entries(rule.components)) { + if (rules.invokes) { + extraImports[dasherizeComponent(componentName, rule)] = { + dependsOnComponents: Object.values(rules.invokes) + .flat() + .map(c => dasherizeComponent(c, rules)), + }; + } + } + } + } + return extraImports; +} + +function dasherizeComponent( + componentSnippet: string, + rules: ModuleRules | ComponentRules | ActivePackageRules +): string { + let d = snippetToDasherizedName(componentSnippet); + if (!d) { + throw new Error( + `unable to parse component snippet "${componentSnippet}" from rule ${JSON.stringify(rules, null, 2)}` + ); + } + return d; +} + function expandDependsOnRules( root: string, filename: string, @@ -145,13 +230,7 @@ function expandDependsOnRules( entry.dependsOnModules = rules.dependsOnModules; } if (rules.dependsOnComponents) { - entry.dependsOnComponents = rules.dependsOnComponents.map(c => { - let d = snippetToDasherizedName(c); - if (!d) { - throw new Error(`unable to parse component snippet "${c}" from rule ${JSON.stringify(rules, null, 2)}`); - } - return d; - }); + entry.dependsOnComponents = rules.dependsOnComponents.map(c => dasherizeComponent(c, rules)); } extraImports[join(root, filename)] = entry; } diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 22000a994..0b7b5b9a6 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -308,6 +308,16 @@ class TemplateResolver implements ASTPlugin { let processedRules = preprocessComponentRule(rules); let dasherizedName = this.standardDasherize(snippet, rule); components.set(dasherizedName, processedRules); + if (rules.layout) { + for (let root of rule.roots) { + if (rules.layout.addonPath) { + files.set(join(root, rules.layout.addonPath), processedRules); + } + if (rules.layout.appPath) { + files.set(join(root, rules.layout.appPath), processedRules); + } + } + } } } if (rule.appTemplates) { diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index e3cf39b5c..1d90d0b2d 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -316,23 +316,21 @@ describe('audit', function () { expect(Object.keys(result.modules).length).toBe(3); }); - test.skip('finds missing component in standalone hbs', async function () { + test('finds missing component in standalone hbs', async function () { merge(app.files, { 'hello.hbs': ``, }); let result = await audit(); expect(withoutCodeFrames(result.findings)).toEqual([ { - message: 'Missing component', - detail: 'NoSuchThing', + message: 'unable to resolve dependency', + detail: '#embroider_compat/components/no-such-thing', filename: './hello.hbs', }, ]); - expect(result.findings[0].codeFrame).toBeDefined(); - expect(Object.keys(result.modules).length).toBe(3); }); - test.skip('finds missing component in inline hbs', async function () { + test('finds missing component in inline hbs', async function () { merge(app.files, { 'app.js': ` import { hbs } from 'ember-cli-htmlbars'; @@ -342,16 +340,14 @@ describe('audit', function () { let result = await audit(); expect(withoutCodeFrames(result.findings)).toEqual([ { - message: 'Missing component', - detail: 'NoSuchThing', + message: 'unable to resolve dependency', + detail: '#embroider_compat/components/no-such-thing', filename: './app.js', }, ]); - expect(result.findings[0].codeFrame).toBeDefined(); - expect(Object.keys(result.modules).length).toBe(2); }); - test.skip('traverse through template even when it has some errors', async function () { + test('traverse through template even when it has some errors', async function () { merge(app.files, { 'hello.hbs': ``, components: { @@ -363,12 +359,12 @@ describe('audit', function () { let result = await audit(); expect(withoutCodeFrames(result.findings)).toEqual([ { - message: 'Missing component', - detail: 'NoSuchThing', + message: 'unable to resolve dependency', + detail: '#embroider_compat/components/no-such-thing', filename: './hello.hbs', }, ]); - expect(Object.keys(result.modules).length).toBe(4); + expect(Object.keys(result.modules)).toContain('./components/second.js'); }); test('failure to parse JS is reported and does not cause cascading errors', async function () { diff --git a/packages/shared-internals/src/index.ts b/packages/shared-internals/src/index.ts index 7e135dfc5..5f91776c5 100644 --- a/packages/shared-internals/src/index.ts +++ b/packages/shared-internals/src/index.ts @@ -1,5 +1,5 @@ export { AppMeta, AddonMeta, PackageInfo } from './metadata'; -export { explicitRelative, extensionsPattern } from './paths'; +export { explicitRelative, extensionsPattern, unrelativize } from './paths'; export { getOrCreate } from './get-or-create'; export { default as Package, V2AddonPackage as AddonPackage, V2AppPackage as AppPackage, V2Package } from './package'; export { default as PackageCache } from './package-cache'; diff --git a/packages/shared-internals/src/paths.ts b/packages/shared-internals/src/paths.ts index fb6b18acd..1d315923e 100644 --- a/packages/shared-internals/src/paths.ts +++ b/packages/shared-internals/src/paths.ts @@ -1,4 +1,5 @@ -import { relative, isAbsolute, dirname, join, basename } from 'path'; +import { relative, isAbsolute, dirname, join, basename, resolve, sep } from 'path'; +import type Package from './package'; // by "explicit", I mean that we want "./local/thing" instead of "local/thing" // because @@ -29,3 +30,14 @@ export function explicitRelative(fromDir: string, toFile: string) { export function extensionsPattern(extensions: string[]): RegExp { return new RegExp(`(${extensions.map(e => `${e.replace('.', '\\.')}`).join('|')})$`, 'i'); } + +export function unrelativize(pkg: Package, specifier: string, fromFile: string) { + if (pkg.packageJSON.exports) { + throw new Error(`unsupported: engines cannot use package.json exports`); + } + let result = resolve(dirname(fromFile), specifier).replace(pkg.root, pkg.name); + if (sep !== '/') { + result = result.split(sep).join('/'); + } + return result; +} diff --git a/tests/scenarios/compat-resolver-test.ts b/tests/scenarios/compat-resolver-test.ts index 2232e06a7..31d07b88d 100644 --- a/tests/scenarios/compat-resolver-test.ts +++ b/tests/scenarios/compat-resolver-test.ts @@ -10,7 +10,7 @@ import { Project, Scenarios } from 'scenario-tester'; import { CompatResolverOptions } from '@embroider/compat/src/resolver-transform'; import { PackageRules } from '@embroider/compat'; -const { module: Qmodule, test, skip } = QUnit; +const { module: Qmodule, test } = QUnit; Scenarios.fromProject(() => new Project()) .map('compat-resolver-test', app => { @@ -1887,7 +1887,7 @@ Scenarios.fromProject(() => new Project()) ); }); - skip('respects invokes rule on a component', async function () { + test('respects invokes rule on a component', async function () { givenFiles({ 'components/my-thing.hbs': `{{component this.which}}`, }); @@ -1906,9 +1906,9 @@ Scenarios.fromProject(() => new Project()) expectTranspiled('components/my-thing.hbs').equalsCode(` window.define("my-app/components/alpha", function () { - return _ref0; + return importSync("#embroider_compat/components/alpha"); }); - import _ref0 from "#embroider_compat/components/alpha"; + import { importSync } from "@embroider/macros"; import { precompileTemplate } from "@ember/template-compilation"; export default precompileTemplate("{{component this.which}}", { moduleName: "my-app/components/my-thing.hbs" diff --git a/tests/scenarios/compat-stage2-test.ts b/tests/scenarios/compat-stage2-test.ts index 69e64c484..d8c715cb1 100644 --- a/tests/scenarios/compat-stage2-test.ts +++ b/tests/scenarios/compat-stage2-test.ts @@ -12,7 +12,7 @@ import merge from 'lodash/merge'; import QUnit from 'qunit'; import { setupAuditTest } from '@embroider/test-support/audit-assertions'; -const { module: Qmodule, test, skip } = QUnit; +const { module: Qmodule, test } = QUnit; let stage2Scenarios = appScenarios.map('compat-stage2-build', app => { renameApp(app, 'my-app'); @@ -467,7 +467,7 @@ stage2Scenarios let expectAudit = setupAuditTest(hooks, () => app.dir); - skip('no audit issues', function () { + test('no audit issues', function () { // among other things, this is asserting that dynamicComponent in // hello-world.hbs is not an error because the rules covered it expectAudit.hasNoFindings(); @@ -511,21 +511,36 @@ stage2Scenarios .isTemplateOnlyComponent('./templates/components/first-choice.hbs'); }); - skip('addon/hello-world.js', function () { - let assertFile = expectFile('node_modules/my-addon/components/hello-world.js').transform(build.transpile); - assertFile.matches( - /window\.define\(["']\my-addon\/synthetic-import-1["'],\s*function\s\(\)\s*\{\s*return\s+esc\(require\(["']\.\.\/synthetic-import-1/ - ); - assertFile.matches( - /window\.define\(["']my-app\/templates\/components\/second-choice["'],\s*function\s\(\)\s*\{\s*return\s+esc\(require\(["']\.\.\/\.\.\/\.\.\/templates\/components\/second-choice\.hbs["']/ - ); + test('addon/hello-world.js', function () { + expectAudit.module('./node_modules/my-addon/components/hello-world.js').codeEquals(` + window.define("my-app/components/second-choice", function () { + return importSync("#embroider_compat/components/second-choice"); + }); + window.define("my-addon/synthetic-import-1", function () { + return importSync("../synthetic-import-1"); + }); + import { importSync } from "@embroider/macros"; + import Component from '@ember/component'; + import layout from '../templates/components/hello-world'; + import computed from '@ember/object/computed'; + import somethingExternal from 'not-a-resolvable-package'; + export default Component.extend({ + dynamicComponentName: computed('useDynamic', function () { + return this.useDynamic || 'default-dynamic'; + }), + layout + }); + `); }); - skip('app/hello-world.js', function () { - let assertFile = expectFile('./components/hello-world.js').transform(build.transpile); - assertFile.matches( - /window\.define\(["']\my-addon\/synthetic-import-1["'],\s*function\s\(\)\s*\{\s*return\s+esc\(require\(["']\.\.\/node_modules\/my-addon\/synthetic-import-1/ - ); + test('app/hello-world.js', function () { + expectAudit.module('./components/hello-world.js').codeEquals(` + window.define("my-addon/synthetic-import-1", function () { + return importSync("my-addon/synthetic-import-1"); + }); + import { importSync } from '@embroider/macros'; + export { default } from 'my-addon/components/hello-world'; + `); expectAudit .module('./components/hello-world.js')