Simplification of gjs and hbs handling in addon-dev

This simplified the resolution of gjs and hbs files in addon-dev. No custom resolveId is needed. The rules are: - publicEntrypoints are always expressed in terms of JS, and those automatically get mapped to their underlying gjs or template-only-component hbs as needed - relative imports within the addon should use explicit extensions (keeping in mind that a template-only-component is always a ".js" extension, even when the js is automatically generated) - the babel config should explicitly handle extensions .hbs and .gjs. This should be released as breaking to document the need to adjust the babel extensions.
embroider-build · Jul 25, 2023 · f613f51 · f613f51
1 parent 9048919
commit f613f51
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 119 deletions.
diff --git a/packages/addon-dev/package.json b/packages/addon-dev/package.json
@@ -28,7 +28,6 @@
   "dependencies": {
     "@embroider/core": "workspace:^",
     "@rollup/pluginutils": "^4.1.1",
-    "assert-never": "^1.2.1",
     "content-tag": "^1.0.0",
     "fs-extra": "^10.0.0",
     "minimatch": "^3.0.4",

diff --git a/packages/addon-dev/src/rollup-gjs-plugin.ts b/packages/addon-dev/src/rollup-gjs-plugin.ts
@@ -1,5 +1,5 @@
 import { createFilter } from '@rollup/pluginutils';
-import type { Plugin, PluginContext, ResolvedId } from 'rollup';
+import type { Plugin } from 'rollup';
 import { readFileSync } from 'fs';
 import { Preprocessor } from 'content-tag';
 
@@ -11,73 +11,18 @@ const processor = new Preprocessor();
 export default function rollupGjsPlugin(): Plugin {
   return {
     name: PLUGIN_NAME,
-    async resolveId(source: string, importer: string | undefined, options) {
-      let resolution = await this.resolve(source, importer, {
-        skipSelf: true,
-        ...options,
-      });
-
-      if (resolution) {
-        return maybeRewriteGJS(resolution);
-      }
-    },
 
     load(id: string) {
-      const meta = getMeta(this, id);
-      if (!meta) {
-        return;
+      if (!gjsFilter(id)) {
+        return null;
       }
-
-      this.addWatchFile(meta.originalId);
-      let input = readFileSync(meta.originalId, 'utf8');
-      let code = processor.process(input);
+      let input = readFileSync(id, 'utf8');
+      let code = processor.process(input, id);
       return {
         code,
       };
     },
   };
 }
 
-type Meta = {
-  originalId: string;
-};
-
-function getMeta(context: PluginContext, id: string): Meta | null {
-  const meta = context.getModuleInfo(id)?.meta?.[PLUGIN_NAME];
-  if (meta) {
-    return meta as Meta;
-  } else {
-    return null;
-  }
-}
-
 const gjsFilter = createFilter('**/*.g{j,t}s');
-
-function maybeRewriteGJS(resolution: ResolvedId) {
-  if (!gjsFilter(resolution.id)) {
-    return null;
-  }
-
-  let id;
-
-  if (resolution.id.endsWith('.gjs')) {
-    id = resolution.id.replace(/\.gjs$/, '.js');
-  } else if (resolution.id.endsWith('.gts')) {
-    id = resolution.id.replace(/\.gts$/, '.ts');
-  } else {
-    throw new Error(
-      'Unexpected issues in the plugin-rollup-gjs - an unexpected file made its way throught the pluginUtils filter'
-    );
-  }
-
-  // This creates an `*.js` or `*.ts` that **replaces** the .gjs or .gts file that we will populate in `load()` hook.
-  return {
-    ...resolution,
-    id,
-    meta: {
-      [PLUGIN_NAME]: {
-        originalId: resolution.id,
-      },
-    },
-  };
-}
diff --git a/packages/addon-dev/src/rollup-hbs-plugin.ts b/packages/addon-dev/src/rollup-hbs-plugin.ts
@@ -1,13 +1,7 @@
 import { createFilter } from '@rollup/pluginutils';
-import type {
-  Plugin,
-  PluginContext,
-  CustomPluginOptions,
-  ResolvedId,
-} from 'rollup';
+import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup';
 import { readFileSync } from 'fs';
 import { hbsToJS } from '@embroider/core';
-import assertNever from 'assert-never';
 import { parse as pathParse } from 'path';
 
 export default function rollupHbsPlugin(): Plugin {
@@ -19,33 +13,25 @@ export default function rollupHbsPlugin(): Plugin {
         ...options,
       });
 
-      if (!resolution) {
-        return maybeSynthesizeComponentJS(this, source, importer, options);
+      if (resolution) {
+        return resolution;
       } else {
-        return maybeRewriteHBS(resolution);
+        return maybeSynthesizeComponentJS(this, source, importer, options);
       }
     },
 
     load(id: string) {
-      const meta = getMeta(this, id);
-      if (!meta) {
-        return;
+      if (hbsFilter(id)) {
+        let input = readFileSync(id, 'utf8');
+        let code = hbsToJS(input);
+        return {
+          code,
+        };
       }
-
-      switch (meta.type) {
-        case 'template':
-          this.addWatchFile(meta.originalId);
-          let input = readFileSync(meta.originalId, 'utf8');
-          let code = hbsToJS(input);
-          return {
-            code,
-          };
-        case 'template-only-component-js':
-          return {
-            code: templateOnlyComponent,
-          };
-        default:
-          assertNever(meta);
+      if (getMeta(this, id)) {
+        return {
+          code: templateOnlyComponent,
+        };
       }
     },
   };
@@ -55,14 +41,9 @@ const templateOnlyComponent =
   `import templateOnly from '@ember/component/template-only';\n` +
   `export default templateOnly();\n`;
 
-type Meta =
-  | {
-      type: 'template';
-      originalId: string;
-    }
-  | {
-      type: 'template-only-component-js';
-    };
+type Meta = {
+  type: 'template-only-component-js';
+};
 
 function getMeta(context: PluginContext, id: string): Meta | null {
   const meta = context.getModuleInfo(id)?.meta?.['rollup-hbs-plugin'];
@@ -108,21 +89,3 @@ async function maybeSynthesizeComponentJS(
 }
 
 const hbsFilter = createFilter('**/*.hbs');
-
-function maybeRewriteHBS(resolution: ResolvedId) {
-  if (!hbsFilter(resolution.id)) {
-    return null;
-  }
-
-  // This creates an `*.hbs.js` that we will populate in `load()` hook.
-  return {
-    ...resolution,
-    id: resolution.id + '.js',
-    meta: {
-      'rollup-hbs-plugin': {
-        type: 'template',
-        originalId: resolution.id,
-      },
-    },
-  };
-}
diff --git a/tests/scenarios/v2-addon-dev-test.ts b/tests/scenarios/v2-addon-dev-test.ts
@@ -76,7 +76,7 @@ appScenarios
             addon.gjs(),
             addon.dependencies(),
 
-            babel({ babelHelpers: 'bundled' }),
+            babel({ babelHelpers: 'bundled', extensions: ['.js', '.hbs', '.gjs'] }),
 
             addon.clean(),
           ],
@@ -101,12 +101,16 @@ appScenarios
       src: {
         components: {
           'single-file-component.gjs': `import Component from '@glimmer/component';
+          import Button from './demo/button.js';
+          import Another from './another.gjs';
           export default class SingleFileComponent extends Component {
-            <template><div data-test-single-file-component>Hello {{@message}}</div></template>
+            <template><div data-test-single-file-component>Hello {{@message}}</div><div data-test-another><Another /></div><Button data-test-button @onClick={{this.doIt}} /></template>
+            doIt = () => {}
           }`,
+          'another.gjs': `<template>Another GJS</template>`,
           demo: {
             'button.hbs': `
-              <button {{on 'click' @onClick}}>
+              <button ...attributes {{on 'click' @onClick}}>
                 flip
               </button>
             `,
@@ -182,7 +186,9 @@ appScenarios
             test('<SingleFileComponent @message="bob" />', async function(assert) {
               await render(hbs\`<SingleFileComponent @message="bob" />\`);
 
-              assert.dom().containsText('Hello bob');
+              assert.dom('[data-test-single-file-component]').containsText('Hello bob');
+              assert.dom('[data-test-another]').containsText('Another GJS');
+              assert.dom('[data-test-button]').containsText('flip');
             })
 
             test('transform worked', async function (assert) {