Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore component invokes rules support #1372

Merged
merged 3 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/compat/src/audit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@ interface InternalModule {
isCJS: boolean;
isAMD: boolean;
dependencies: string[];
transpiledContent: string | Buffer;
};

resolved?: Map<string, string | ResolutionFailure>;

content?: string | Buffer;

linked?: {
exports: Set<string>;
};
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -332,7 +331,6 @@ export class Audit {
} else {
module.parsed = visitResult;
module.resolved = await this.resolveDeps(visitResult.dependencies, filename);
module.content = content;
}
}
}
Expand Down Expand Up @@ -479,6 +477,7 @@ export class Audit {
isCJS: false,
isAMD: false,
dependencies,
transpiledContent: content,
};
}

Expand All @@ -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)) {
Expand Down
6 changes: 3 additions & 3 deletions packages/compat/src/audit/babel-visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<t.Identifier>) {
if (path.node.name === 'module' && isFreeVariable(path)) {
sawModule = true;
Expand Down Expand Up @@ -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 {
Expand Down
159 changes: 119 additions & 40 deletions packages/compat/src/babel-plugin-adjust-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand All @@ -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) {
Expand All @@ -39,6 +47,7 @@ export default function main(babel: typeof Babel) {
resolverOptions,
resolver: new Resolver(resolverOptions),
extraImports: preprocessExtraImports(resolverOptions),
componentExtraImports: preprocessComponentExtraImports(resolverOptions),
};
return cached;
}
Expand All @@ -61,35 +70,57 @@ export default function main(babel: typeof Babel) {
function addExtraImports(t: BabelTypes, path: NodePath<t.Program>, 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<t.Program>,
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<t.Program>, target: string, runtimeName: string) {
Expand All @@ -102,8 +133,8 @@ function amdDefine(t: BabelTypes, adder: ImportUtil, path: NodePath<t.Program>,
);
}

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)) {
Expand Down Expand Up @@ -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<Resolver['owningEngine']> | 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,
Expand All @@ -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;
}
Expand Down
10 changes: 10 additions & 0 deletions packages/compat/src/resolver-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
24 changes: 10 additions & 14 deletions packages/compat/tests/audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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': `<NoSuchThing />`,
});
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';
Expand All @@ -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': `<NoSuchThing /><Second />`,
components: {
Expand All @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion packages/shared-internals/src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
14 changes: 13 additions & 1 deletion packages/shared-internals/src/paths.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
Loading