From 7d8b1d7d8b774b0b3f08027c5ec742491b1616eb Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 6 Jun 2023 12:38:59 -0400 Subject: [PATCH 1/3] failing test for hbs in app tree --- tests/scenarios/core-resolver-test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/scenarios/core-resolver-test.ts b/tests/scenarios/core-resolver-test.ts index d2da534fc..f8fec9930 100644 --- a/tests/scenarios/core-resolver-test.ts +++ b/tests/scenarios/core-resolver-test.ts @@ -477,6 +477,24 @@ Scenarios.fromProject(() => new Project()) .to('./node_modules/my-addon/_app_/hello-world.js'); }); + test('hbs in addon is found', async function () { + givenFiles({ + 'node_modules/my-addon/_app_/templates/hello-world.hbs': '', + 'app.js': `import "my-app/templates/hello-world"`, + }); + + await configure({ + addonMeta: { + 'app-js': { './templates/hello-world.hbs': './_app_/templates/hello-world.hbs' }, + }, + }); + + expectAudit + .module('./app.js') + .resolves('my-app/templates/hello-world') + .to('./node_modules/my-addon/_app_/templates/hello-world.hbs'); + }); + test(`relative import in addon's app tree resolves to app`, async function () { givenFiles({ 'node_modules/my-addon/_app_/hello-world.js': `import "./secondary"`, From d6c5515b48df94228ed471491eef41fb28b4db4c Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Mon, 12 Jun 2023 19:50:04 +0100 Subject: [PATCH 2/3] look up extensionless module specifiers --- packages/core/src/module-resolver.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 764333b38..0f0e41e00 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -306,7 +306,7 @@ export class Resolver { let pkg = this.owningPackage(match.filename); if (pkg) { - let rel = withoutJSExt(explicitRelative(pkg.root, match.filename)); + let rel = explicitRelative(pkg.root, match.filename); let entry = this.mergeMap.get(pkg.root)?.get(rel); if (entry?.type === 'both') { return request.alias(entry[section].localPath).rehome(resolve(entry[section].packageRoot, 'package.json')); @@ -504,7 +504,6 @@ export class Resolver { `addon ${addon.name} declares app-js in its package.json with the illegal name "${inAddonName}". It must start with "./" to make it clear that it's relative to the addon` ); } - inEngineName = withoutJSExt(inEngineName); let prevEntry = engineModules.get(inEngineName); switch (prevEntry?.type) { case undefined: @@ -549,7 +548,6 @@ export class Resolver { `addon ${addon.name} declares fastboot-js in its package.json with the illegal name "${inAddonName}". It must start with "./" to make it clear that it's relative to the addon` ); } - inEngineName = withoutJSExt(inEngineName); let prevEntry = engineModules.get(inEngineName); switch (prevEntry?.type) { case undefined: @@ -933,8 +931,17 @@ export class Resolver { engine: EngineConfig, inEngineSpecifier: string ): R | undefined { - inEngineSpecifier = withoutJSExt(inEngineSpecifier); - let entry = this.mergeMap.get(engine.root)?.get(inEngineSpecifier); + let entry; + + if (inEngineSpecifier.match(/\.(hbs|js|hbs\.js)$/)) { + entry = this.mergeMap.get(engine.root)?.get(inEngineSpecifier); + } else { + // try looking up .hbs .js and .hbs.js in that order for specifiers without extenstions + ['.hbs', '.js', '.hbs.js'].find(ext => { + return (entry = this.mergeMap.get(engine.root)?.get(`${inEngineSpecifier}${ext}`)); + }); + } + switch (entry?.type) { case undefined: return undefined; @@ -1060,11 +1067,3 @@ function external(label: string, request: R, specifier: let filename = virtualExternalModule(specifier); return logTransition(label, request, request.virtualize(filename)); } - -// this is specifically for app-js handling, where only .js and .hbs are legal -// extensiosn, and only .js is allowed to be an *implied* extension (.hbs must -// be explicit). So when normalizing such paths, it's only a .js suffix that we -// must remove. -function withoutJSExt(filename: string): string { - return filename.replace(/\.js$/, ''); -} From d903c018557025a20f0c4087efb47b09f51a40d7 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Tue, 13 Jun 2023 10:37:06 +0100 Subject: [PATCH 3/3] create getEntryFromMergeMap helper --- packages/core/src/module-resolver.ts | 29 ++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 0f0e41e00..e5b3febc0 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -307,7 +307,7 @@ export class Resolver { let pkg = this.owningPackage(match.filename); if (pkg) { let rel = explicitRelative(pkg.root, match.filename); - let entry = this.mergeMap.get(pkg.root)?.get(rel); + let entry = this.getEntryFromMergeMap(rel, pkg.root); if (entry?.type === 'both') { return request.alias(entry[section].localPath).rehome(resolve(entry[section].packageRoot, 'package.json')); } @@ -926,21 +926,30 @@ export class Resolver { return logTransition('fallbackResolve final exit', request); } - private searchAppTree( - request: R, - engine: EngineConfig, - inEngineSpecifier: string - ): R | undefined { - let entry; + private getEntryFromMergeMap(inEngineSpecifier: string, root: string): MergeEntry | undefined { + let entry: MergeEntry | undefined; if (inEngineSpecifier.match(/\.(hbs|js|hbs\.js)$/)) { - entry = this.mergeMap.get(engine.root)?.get(inEngineSpecifier); + entry = this.mergeMap.get(root)?.get(inEngineSpecifier); } else { // try looking up .hbs .js and .hbs.js in that order for specifiers without extenstions - ['.hbs', '.js', '.hbs.js'].find(ext => { - return (entry = this.mergeMap.get(engine.root)?.get(`${inEngineSpecifier}${ext}`)); + ['.hbs', '.js', '.hbs.js'].forEach(ext => { + if (entry) { + return; + } + + entry = this.mergeMap.get(root)?.get(`${inEngineSpecifier}${ext}`); }); } + return entry; + } + + private searchAppTree( + request: R, + engine: EngineConfig, + inEngineSpecifier: string + ): R | undefined { + let entry = this.getEntryFromMergeMap(inEngineSpecifier, engine.root); switch (entry?.type) { case undefined: