Merge pull request #1463 from embroider-build/hbs-in-app-tree

fix resolution of files with .hbs extensions
embroider-build · Jun 13, 2023 · 5a5f4d5 · 5a5f4d5
2 parents 95eef86 + d903c01
commit 5a5f4d5
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 14 deletions.
diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts
@@ -306,8 +306,8 @@ export class Resolver {
 
     let pkg = this.owningPackage(match.filename);
     if (pkg) {
-      let rel = withoutJSExt(explicitRelative(pkg.root, match.filename));
-      let entry = this.mergeMap.get(pkg.root)?.get(rel);
+      let rel = explicitRelative(pkg.root, match.filename);
+      let entry = this.getEntryFromMergeMap(rel, pkg.root);
       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:
@@ -928,13 +926,31 @@ export class Resolver {
     return logTransition('fallbackResolve final exit', request);
   }
 
+  private getEntryFromMergeMap(inEngineSpecifier: string, root: string): MergeEntry | undefined {
+    let entry: MergeEntry | undefined;
+
+    if (inEngineSpecifier.match(/\.(hbs|js|hbs\.js)$/)) {
+      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'].forEach(ext => {
+        if (entry) {
+          return;
+        }
+
+        entry = this.mergeMap.get(root)?.get(`${inEngineSpecifier}${ext}`);
+      });
+    }
+    return entry;
+  }
+
   private searchAppTree<R extends ModuleRequest>(
     request: R,
     engine: EngineConfig,
     inEngineSpecifier: string
   ): R | undefined {
-    inEngineSpecifier = withoutJSExt(inEngineSpecifier);
-    let entry = this.mergeMap.get(engine.root)?.get(inEngineSpecifier);
+    let entry = this.getEntryFromMergeMap(inEngineSpecifier, engine.root);
+
     switch (entry?.type) {
       case undefined:
         return undefined;
@@ -1060,11 +1076,3 @@ function external<R extends ModuleRequest>(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$/, '');
-}
diff --git 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"`,