Compile Hbs route templates correctly

docs/v2-faq.md

@@ -17,6 +17,7 @@
   * [Why do v2 addons need a build step?](#why-do-v2-addons-need-a-build-step)
   * [How can I integrate with the app's build?](#how-can-i-integrate-with-the-apps-build)
   * [How can I define the public exports of my addon?](#how-can-i-define-the-public-exports-of-my-addon)
+  * [How can I provide route templates with my v2 addon?](#how-can-i-provide-route-templates-with-my-v2-addon)
 
 <!-- tocstop -->
 
@@ -195,3 +196,29 @@ Additionally, there is a feature supported in node.js and modern bundlers to def
 When using `package.json#exports` make sure that:
 - the `addon.publicEntrypoints(...)` plugin in `rollup.config.mjs` includes _at least_ whatever is defined in `package.json#exports`
 - the modules that `addon.appReexports(...)` exposes must have overlap with the `package.json#exports` so that the app-tree merging may import from the addon
+
+### How can I provide route templates with my v2 addon?
+
+During a v2 addon build step, standalone `.hbs` are considered template-only components by default.
+
+If you want your v2 addon to provide a route template, the best way to proceed is to make it a `.gjs` file using [ember-route-template](https://github.com/discourse/ember-route-template). Similarly, if you want to migrate to v2 a classic addon that used to provide `.hbs` route templates, you should refactor the `.hbs` to `.gjs` files to complete the migration.
+
+If for whatever reason the `.gjs` approach cannot be used, it's still possible to have your v2 addon providing the route templates as `.hbs`, but it requires extra configuration. During the build step, Rollup and Babel work together to transform all standalone `.hbs` into template-only components. Therefore, you need to tell both Rollup and Babel to _not_ compile a given list of `.hbs` files this way.
+
+Let's assume your addon has a `templates/` folder that contains all your route templates. The files in `templates/` should be compiled as simple templates (not template-only components).
+
+In the `rollup.config.mjs`, pass a list of glob patterns in the `excludeColocation` option of the function `addon.hbs`:
+
+```js 
+addon.hbs({ excludeColocation: ['templates/**/*'] }),
+```
+
+In the `babel.config.json`, pass the same list of glob patterns in the `exclude` option of the `template-colocation-plugin`:
+
+```
+"plugins": [
+  ["@embroider/addon-dev/template-colocation-plugin", {
+    exclude: ['templates/**/*']
+  }],
+],
+```

packages/addon-dev/src/rollup-hbs-plugin.ts

@@ -1,10 +1,14 @@
 import { createFilter } from '@rollup/pluginutils';
 import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup';
 import { readFileSync } from 'fs';
-import { hbsToJS } from '@embroider/core';
-import { parse as pathParse } from 'path';
+import { correspondingTemplate, hbsToJS } from '@embroider/core';
+import minimatch from 'minimatch';
 
-export default function rollupHbsPlugin(): Plugin {
+export default function rollupHbsPlugin({
+  excludeColocation,
+}: {
+  excludeColocation?: string[];
+}): Plugin {
   return {
     name: 'rollup-hbs-plugin',
     async resolveId(source: string, importer: string | undefined, options) {
@@ -16,19 +20,26 @@ export default function rollupHbsPlugin(): Plugin {
       if (resolution) {
         return resolution;
       } else {
-        return maybeSynthesizeComponentJS(this, source, importer, options);
+        return maybeSynthesizeComponentJS(
+          this,
+          source,
+          importer,
+          options,
+          excludeColocation
+        );
       }
     },
 
     load(id: string) {
       if (hbsFilter(id)) {
-        let input = readFileSync(id, 'utf8');
-        let code = hbsToJS(input);
-        return {
-          code,
-        };
+        return getHbsToJSCode(id);
       }
-      if (getMeta(this, id)) {
+      let meta = getMeta(this, id);
+      if (meta) {
+        if (meta?.type === 'template-js') {
+          const hbsFile = id.replace(/\.js$/, '.hbs');
+          return getHbsToJSCode(hbsFile);
+        }
         return {
           code: templateOnlyComponent,
         };
@@ -42,7 +53,7 @@ const templateOnlyComponent =
   `export default templateOnly();\n`;
 
 type Meta = {
-  type: 'template-only-component-js';
+  type: 'template-only-component-js' | 'template-js';
 };
 
 function getMeta(context: PluginContext, id: string): Meta | null {
@@ -54,35 +65,40 @@ function getMeta(context: PluginContext, id: string): Meta | null {
   }
 }
 
-function correspondingTemplate(filename: string): string {
-  let { ext } = pathParse(filename);
-  return filename.slice(0, filename.length - ext.length) + '.hbs';
+function getHbsToJSCode(file: string): { code: string } {
+  let input = readFileSync(file, 'utf8');
+  let code = hbsToJS(input);
+  return {
+    code,
+  };
 }
 
 async function maybeSynthesizeComponentJS(
   context: PluginContext,
   source: string,
   importer: string | undefined,
-  options: { custom?: CustomPluginOptions; isEntry: boolean }
+  options: { custom?: CustomPluginOptions; isEntry: boolean },
+  excludeColocation: string[] | undefined
 ) {
-  let templateResolution = await context.resolve(
-    correspondingTemplate(source),
-    importer,
-    {
-      skipSelf: true,
-      ...options,
-    }
-  );
+  let hbsFilename = correspondingTemplate(source);
+  let templateResolution = await context.resolve(hbsFilename, importer, {
+    skipSelf: true,
+    ...options,
+  });
   if (!templateResolution) {
     return null;
   }
+  let type = excludeColocation?.some((glob) => minimatch(hbsFilename, glob))
+    ? 'template-js'
+    : 'template-only-component-js';
   // we're trying to resolve a JS module but only the corresponding HBS
-  // file exists. Synthesize the template-only component JS.
+  // file exists. Synthesize the JS. The meta states if the hbs corresponds
+  // to a template-only component or a simple template like a route template.
   return {
     id: templateResolution.id.replace(/\.hbs$/, '.js'),
     meta: {
       'rollup-hbs-plugin': {
-        type: 'template-only-component-js',
+        type,
       },
     },
   };

packages/addon-dev/src/rollup.ts

@@ -53,8 +53,8 @@ export class Addon {
   // This wraps standalone .hbs files as Javascript files using inline
   // templates. This means special resolving rules for .hbs files aren't
   // required for javascript tooling to understand your package.
-  hbs() {
-    return hbs();
+  hbs(options = {}) {
+    return hbs(options);
   }
 
   gjs(options?: { inline_source_map: boolean }) {

packages/shared-internals/package.json

@@ -36,6 +36,7 @@
     "typescript-memoize": "^1.0.1",
     "fs-extra": "^9.1.0",
     "lodash": "^4.17.21",
+    "minimatch": "^3.0.4",
     "semver": "^7.3.5"
   },
   "devDependencies": {
@@ -47,6 +48,7 @@
     "@types/fs-extra": "^9.0.12",
     "@types/lodash": "^4.14.170",
     "@types/js-string-escape": "^1.0.0",
+    "@types/minimatch": "^3.0.4",
     "@types/semver": "^7.3.6",
     "@types/tmp": "^0.1.0",
     "fixturify": "^2.1.1",

packages/shared-internals/src/index.ts

@@ -1,5 +1,5 @@
 export { AppMeta, AddonMeta, PackageInfo } from './metadata';
-export { explicitRelative, extensionsPattern, unrelativize, cleanUrl } from './paths';
+export { explicitRelative, extensionsPattern, unrelativize, cleanUrl, correspondingTemplate } 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';

packages/shared-internals/src/paths.ts

@@ -1,4 +1,4 @@
-import { relative, isAbsolute, dirname, join, basename, resolve, sep } from 'path';
+import { relative, isAbsolute, dirname, join, basename, resolve, sep, parse as pathParse } from 'path';
 import type Package from './package';
 
 // by "explicit", I mean that we want "./local/thing" instead of "local/thing"
@@ -49,3 +49,10 @@ const postfixRE = /[?#].*$/s;
 export function cleanUrl(url: string): string {
   return url.replace(postfixRE, '');
 }
+
+// given a filename, returns it with the hbs extension
+// for instance, passing filename.js returns filename.hbs
+export function correspondingTemplate(filename: string): string {
+  let { ext } = pathParse(filename);
+  return filename.slice(0, filename.length - ext.length) + '.hbs';
+}

packages/shared-internals/src/template-colocation-plugin.ts

@@ -6,7 +6,8 @@ import { dirname } from 'path';
 import { explicitRelative, PackageCache } from '.';
 import { ImportUtil } from 'babel-import-util';
 import makeDebug from 'debug';
-import { cleanUrl } from './paths';
+import minimatch from 'minimatch';
+import { cleanUrl, correspondingTemplate } from './paths';
 
 const debug = makeDebug('embroider:template-colocation-plugin');
 
@@ -36,6 +37,14 @@ export interface Options {
   // This option is used by Embroider itself to help with v1 addon
   // compatibility, other users should probably not use it.
   templateExtensions?: string[];
+
+  // Default to []
+  //
+  // Skip the plugin for files that match the specified globs.
+  //
+  // This option is used to prevent the plugin to transform the
+  // compiled output of hbs files that are not colocated components.
+  exclude?: string[];
 }
 
 interface State {
@@ -66,6 +75,11 @@ export default function main(babel: typeof Babel) {
             }
           }
 
+          if (state.opts.exclude?.some(glob => minimatch(correspondingTemplate(filename), glob))) {
+            debug('not handling colocation for %s', filename);
+            return;
+          }
+
           debug('handling colocation for %s', filename);
           let extensions = state.opts.templateExtensions ?? ['.hbs'];
           for (let ext of extensions) {

tests/scenarios/v2-addon-dev-test.ts

@@ -29,7 +29,9 @@ appScenarios
       'babel.config.json': `
         {
           "plugins": [
-            "@embroider/addon-dev/template-colocation-plugin",
+            ["@embroider/addon-dev/template-colocation-plugin", {
+              exclude: ['**/just-a-template.hbs'],
+            }],
             "@babel/plugin-transform-class-static-block",
             ["babel-plugin-ember-template-compilation", {
               targetFormat: 'hbs',
@@ -71,7 +73,9 @@ appScenarios
               exclude: ['**/-excluded/**/*'],
             }),
 
-            addon.hbs(),
+            addon.hbs({
+              excludeColocation: ['**/just-a-template.hbs'],
+            }),
             addon.gjs(),
             addon.dependencies(),
 
@@ -113,6 +117,7 @@ appScenarios
                 flip
               </button>
             `,
+            'just-a-template.hbs': `<p>I am not a component but a template.</p>`,
             'out.hbs': `
               <out>{{yield}}</out>
             `,
@@ -244,13 +249,15 @@ appScenarios
             './components/demo/button.js': './dist/_app_/components/demo/button.js',
             './components/single-file-component.js': './dist/_app_/components/single-file-component.js',
             './components/demo/index.js': './dist/_app_/components/demo/index.js',
+            './components/demo/just-a-template.js': './dist/_app_/components/demo/just-a-template.js',
             './components/demo/out.js': './dist/_app_/components/demo/out.js',
             './components/demo/namespace/namespace-me.js': './dist/_app_/components/demo/namespace/namespace-me.js',
           });
         });
 
         test('the addon has expected public entrypoints', async function () {
           expectFile('dist/components/demo/index.js').exists();
+          expectFile('dist/components/demo/just-a-template.js').exists();
           expectFile('dist/components/demo/out.js').exists();
           expectFile('dist/components/demo/namespace-me.js').exists();
           expectFile('dist/components/-excluded/never-import-this.js').doesNotExist();
@@ -260,6 +267,9 @@ appScenarios
           expectFile('dist/_app_/components/demo/index.js').matches(
             'export { default } from "v2-addon/components/demo/index"'
           );
+          expectFile('dist/_app_/components/demo/just-a-template.js').matches(
+            'export { default } from "v2-addon/components/demo/just-a-template"'
+          );
           expectFile('dist/_app_/components/demo/out.js').matches(
             'export { default } from "v2-addon/components/demo/out"'
           );
@@ -274,6 +284,13 @@ appScenarios
             /TEMPLATE = precompileTemplate\("Hello there/,
             'template is still in hbs format'
           );
+
+          expectFile(
+            'dist/components/demo/just-a-template.js'
+          ).equalsCode(`import { precompileTemplate } from '@ember/template-compilation';
+var justATemplate = precompileTemplate("<p>I am not a component but a template.</p>");
+export { justATemplate as default };
+//# sourceMappingURL=just-a-template.js.map`);
         });
 
         test('gjs components compiled correctly', async function () {