Skip to content

Commit

Permalink
fix colocation updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@patricklx
patricklx committed Jun 17, 2024
1 parent 5efaa0e commit 3b98560
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 40 deletions.
49 changes: 31 additions & 18 deletions packages/addon-dev/src/rollup-hbs-plugin.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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';

Expand Down Expand Up @@ -29,34 +30,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 {
Expand All @@ -68,7 +80,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,
Expand Down Expand Up @@ -97,7 +110,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,
Expand Down
7 changes: 0 additions & 7 deletions packages/addon-dev/src/rollup-incremental-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
51 changes: 36 additions & 15 deletions tests/scenarios/v2-addon-dev-watch-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down

0 comments on commit 3b98560

Please sign in to comment.