From 0106e1eac5f5e5e0585711ee7d068f89508e2fe1 Mon Sep 17 00:00:00 2001 From: Patrick Pircher Date: Fri, 14 Jun 2024 17:26:26 +0200 Subject: [PATCH] fix colocation updates --- packages/addon-dev/src/rollup-hbs-plugin.ts | 50 +++++++++++------- .../src/rollup-incremental-plugin.ts | 7 --- tests/scenarios/v2-addon-dev-watch-test.ts | 51 +++++++++++++------ 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/packages/addon-dev/src/rollup-hbs-plugin.ts b/packages/addon-dev/src/rollup-hbs-plugin.ts index edf90249a..f82a5a726 100644 --- a/packages/addon-dev/src/rollup-hbs-plugin.ts +++ b/packages/addon-dev/src/rollup-hbs-plugin.ts @@ -1,7 +1,9 @@ import { createFilter } from '@rollup/pluginutils'; import type { Plugin, PluginContext, CustomPluginOptions } from 'rollup'; +import { readFileSync } from 'fs'; import { correspondingTemplate, hbsToJS } from '@embroider/core'; import minimatch from 'minimatch'; +import { existsSync } from 'fs-extra'; export default function rollupHbsPlugin({ excludeColocation, @@ -29,34 +31,45 @@ export default function rollupHbsPlugin({ } }, - async transform(code: string, id: string) { - if (!hbsFilter(id)) { - return; + transform(code: string, id: string) { + let hbsFilename = id.replace(/\.\w{1,3}$/, '') + '.hbs'; + if (hbsFilename !== id) { + this.addWatchFile(hbsFilename); + } + if (hbsFilter(id)) { + let basename = id.replace(/\.\w{1,3}$/, ''); + this.addWatchFile(basename + '.ts'); + this.addWatchFile(basename + '.js'); + return { + code: hbsToJS(code), + }; + } + }, + + load(id: string) { + if (hbsFilter(id)) { + return null; } let meta = getMeta(this, id); - if (meta?.type === 'template-only-component-js') { + if (meta) { + if (meta?.type === 'template-js') { + const hbsFile = id.replace(/\.js$/, '.hbs'); + return getHbsToJSCode(hbsFile); + } return { - code: templateOnlyComponent(code), + code: templateOnlyComponent, }; } - return getHbsToJSCode(code); }, }; } -function templateOnlyComponent(hbsCode: string) { - const code = hbsCode.replace(/`/g, '\\`').replace(/\$/g, '\\$'); - return ` - import templateOnly from '@ember/component/template-only'; - import { precompileTemplate } from '@ember/template-compilation'; - import { setComponentTemplate } from '@ember/component'; - export default setComponentTemplate(precompileTemplate(\`${code}\`), templateOnly()); - `; -} +const templateOnlyComponent = + `import templateOnly from '@ember/component/template-only';\n` + + `export default templateOnly();\n`; type Meta = { type: 'template-only-component-js' | 'template-js'; - hbsFile: string; }; function getMeta(context: PluginContext, id: string): Meta | null { @@ -68,7 +81,8 @@ function getMeta(context: PluginContext, id: string): Meta | null { } } -function getHbsToJSCode(input: string): { code: string } { +function getHbsToJSCode(file: string): { code: string } { + let input = readFileSync(file, 'utf8'); let code = hbsToJS(input); return { code, @@ -97,7 +111,7 @@ async function maybeSynthesizeComponentJS( // 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, + id: templateResolution.id.replace(/\.hbs$/, '.js'), meta: { 'rollup-hbs-plugin': { type, diff --git a/packages/addon-dev/src/rollup-incremental-plugin.ts b/packages/addon-dev/src/rollup-incremental-plugin.ts index 972205f3e..3f153bbc9 100644 --- a/packages/addon-dev/src/rollup-incremental-plugin.ts +++ b/packages/addon-dev/src/rollup-incremental-plugin.ts @@ -88,13 +88,6 @@ export default function incremental(): Plugin { name: 'clean', transform(_code, id) { changed.add(id); - // support colocation changes - // could also be done directly in the babel plugin - // by passing rollup context into it - let hbsFilename = id.replace(/\.\w{1,3}$/, '') + '.hbs'; - if (hbsFilename !== id && existsSync(hbsFilename)) { - this.addWatchFile(hbsFilename); - } }, generateBundle(options, bundle) { if (firstTime) { diff --git a/tests/scenarios/v2-addon-dev-watch-test.ts b/tests/scenarios/v2-addon-dev-watch-test.ts index 1b1126740..2202659fb 100644 --- a/tests/scenarios/v2-addon-dev-watch-test.ts +++ b/tests/scenarios/v2-addon-dev-watch-test.ts @@ -50,8 +50,8 @@ Scenarios.fromProject(() => baseV2Addon()) output: addon.output(), plugins: [ - addon.publicEntrypoints(['components/**/*.{gts,js}']), - addon.appReexports(['components/**/*.{gts,js}']), + addon.publicEntrypoints(['components/**/*.js']), + addon.appReexports(['components/**/*.js']), addon.clean(), addon.gjs(), addon.hbs(), @@ -142,42 +142,46 @@ Scenarios.fromProject(() => baseV2Addon()) await watcher.start(); - let someFile = path.join(addon.dir, 'src/components/demo.hbs'); + let demoHbs = path.join(addon.dir, 'src/components/demo.hbs'); + let demoJs = path.join(addon.dir, 'src/components/demo.js'); let distPath = path.join(addon.dir, 'dist/components/test.js'); - let srcPathDemo = path.join(addon.dir, 'src/components/demo.hbs'); let distPathDemoComp = path.join(addon.dir, 'dist/components/demo.js'); let srcPathButton = path.join(addon.dir, 'src/components/other.hbs'); let distPathButton = path.join(addon.dir, 'dist/_app_/components/other.js'); assert.strictEqual(existsSync(distPathButton), true, `Expected ${distPathButton} to exist`); let origContent = await fs.readFile(srcPathButton); - let demoContent = await fs.readFile(srcPathDemo); + let demoContent = await fs.readFile(demoHbs); + // deleting a component from src should delete it from dist await fs.rm(srcPathButton); await watcher?.nextBuild(); assert.strictEqual(existsSync(distPathButton), false, `Expected ${distPathButton} to be deleted`); + // create a component in src should create it in dist await fs.writeFile(srcPathButton, origContent); await watcher?.nextBuild(); assert.strictEqual(existsSync(distPathButton), true, `Expected ${distPathButton} to exist`); + // updating hbs modifies colocated js await becomesModified({ filePath: distPathDemoComp, assert, // Update a component fn: async () => { - let someContent = await fs.readFile(srcPathDemo); + let someContent = await fs.readFile(demoHbs); // generally it's bad to introduce time dependencies to a test, but we need to wait long enough // to guess for how long it'll take for the file system to update our file. // // the `stat` is measured in `ms`, so it's still pretty fast await aBit(10); - await fs.writeFile(srcPathDemo, someContent + `\n`); + await fs.writeFile(demoHbs, someContent + `\n`); await watcher?.nextBuild(); }, }); + // removing hbs modifies colocated js to not import hbs anymore await becomesModified({ filePath: distPathDemoComp, assert, @@ -188,65 +192,82 @@ Scenarios.fromProject(() => baseV2Addon()) // // the `stat` is measured in `ms`, so it's still pretty fast await aBit(10); - await fs.rm(srcPathDemo); + await fs.rm(demoHbs); await watcher?.nextBuild(); }, }); - await fs.writeFile(srcPathDemo, demoContent); + await fs.writeFile(demoHbs, demoContent); + await watcher?.nextBuild(); + // updating hbs content should not update unrelated files await isNotModified({ filePath: distPath, assert, // Update a component fn: async () => { - let someContent = await fs.readFile(someFile); + let someContent = await fs.readFile(demoHbs); // generally it's bad to introduce time dependencies to a test, but we need to wait long enough // to guess for how long it'll take for the file system to update our file. // // the `stat` is measured in `ms`, so it's still pretty fast + await fs.writeFile(demoHbs, someContent + `\n\n`); await aBit(10); - await fs.writeFile(someFile, someContent + `\n`); await watcher?.nextBuild(); }, }); + // updating hbs content should update resulting app re-exported component distPath = path.join(addon.dir, 'dist/_app_/components/test.js'); await isNotModified({ filePath: distPath, assert, // Update a component fn: async () => { - let someContent = await fs.readFile(someFile); + let someContent = await fs.readFile(demoHbs); // generally it's bad to introduce time dependencies to a test, but we need to wait long enough // to guess for how long it'll take for the file system to update our file. // // the `stat` is measured in `ms`, so it's still pretty fast await aBit(10); - await fs.writeFile(someFile, someContent + `\n`); + await fs.writeFile(demoHbs, someContent + `\n`); await watcher?.nextBuild(); }, }); + // updating template only hbs should update the dist output distPath = path.join(addon.dir, 'dist/components/button.js'); await isNotModified({ filePath: distPath, assert, // Update a component fn: async () => { - let someContent = await fs.readFile(someFile); + let someContent = await fs.readFile(demoHbs); // generally it's bad to introduce time dependencies to a test, but we need to wait long enough // to guess for how long it'll take for the file system to update our file. // // the `stat` is measured in `ms`, so it's still pretty fast await aBit(10); - await fs.writeFile(someFile, someContent + `\n`); + await fs.writeFile(demoHbs, someContent + `\n`); await watcher?.nextBuild(); }, }); + + // deleting demo.js should make demo a template only component + const demoJsContent = await fs.readFile(demoJs); + await fs.rm(demoJs); + await watcher?.nextBuild(); + let distPathDemoCompContent = await fs.readFile(distPathDemoComp); + assert.ok(distPathDemoCompContent.includes('templateOnly')); + + // creating demo.js should make demo a template colocated component + await fs.writeFile(demoJs, demoJsContent); + await watcher?.nextBuild(); + distPathDemoCompContent = await fs.readFile(distPathDemoComp); + assert.ok(!distPathDemoCompContent.includes('templateOnly')); }); test('the package.json is not updated since it would be the same', async function (assert) {