Skip to content

Commit

Permalink
fix v2 addon watch
Browse files Browse the repository at this point in the history
  • Loading branch information
@patricklx
patricklx committed Dec 17, 2024
1 parent 01e21e6 commit d0f9bc0
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 73 deletions.
11 changes: 10 additions & 1 deletion packages/addon-dev/src/rollup-app-reexports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ import { extname } from 'path';
import minimatch from 'minimatch';
import type { Plugin } from 'rollup';

function symmetricDifference(arr1: any[], arr2: any[]) {
return arr1
.filter((x) => !arr2.includes(x))
.concat(arr2.filter((x) => !arr1.includes(x)));
}

export default function appReexports(opts: {
from: string;
to: string;
Expand Down Expand Up @@ -42,7 +48,10 @@ export default function appReexports(opts: {
}
let originalAppJS = pkg['ember-addon']?.['app-js'];

let hasChanges = JSON.stringify(originalAppJS) !== JSON.stringify(appJS);
let hasChanges = !!symmetricDifference(
Object.keys(originalAppJS),
Object.keys(appJS)
).length;

// Don't cause a file i/o event unless something actually changed
if (hasChanges) {
Expand Down
22 changes: 19 additions & 3 deletions packages/addon-dev/src/rollup-public-entrypoints.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,33 @@ export default function publicEntrypoints(args: {
include: string[];
exclude?: string[];
}): Plugin {
const allEntryPoints: Set<string> = new Set();

return {
name: 'addon-modules',

async buildStart() {
const include = [
...args.include,
'**/*.hbs',
'**/*.ts',
'**/*.gts',
'**/*.gjs',
];
// wait a bit first https://github.com/nodejs/node/issues/4760
await new Promise((resolve) => setTimeout(resolve, 50));
let matches = walkSync(args.srcDir, {
globs: [...args.include, '**/*.hbs', '**/*.ts', '**/*.gts', '**/*.gjs'],
globs: include,
ignore: args.exclude,
});

matches.forEach((m) => allEntryPoints.add(m));

for (const name of allEntryPoints) {
this.addWatchFile(path.resolve(args.srcDir, name));
}

for (let name of matches) {
this.addWatchFile(path.join(args.srcDir, name));
// the matched file, but with the extension swapped with .js
let normalizedName = normalizeFileExt(name);

Expand Down Expand Up @@ -58,7 +74,7 @@ export default function publicEntrypoints(args: {

// additionally, we want to emit chunks where the pattern matches the supported
// file extensions above (TS, GTS, etc) as if they were already the built JS.
let wouldMatchIfBuilt = args.include.some((pattern) =>
let wouldMatchIfBuilt = include.some((pattern) =>
minimatch(normalizedName, pattern)
);

Expand Down
49 changes: 38 additions & 11 deletions tests/scenarios/helpers/v2-addon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import path from 'path';
import type { PreparedApp } from 'scenario-tester';
import { loadConfigFile } from 'rollup/loadConfigFile';
import rollup from 'rollup';
import type { RollupOptions } from 'rollup';
import type { RollupOptions, Plugin } from 'rollup';

export class DevWatcher {
#counter: number;
#expectedBuilds: number;
#addon: PreparedApp;
#watcher?: ReturnType<(typeof rollup)['watch']>;
#waitForBuildPromise?: Promise<unknown>;
Expand All @@ -15,13 +17,17 @@ export class DevWatcher {
constructor(addon: PreparedApp) {
this.#addon = addon;
this.#originalDirectory = process.cwd();
this.#counter = 1;
this.#expectedBuilds = 1;
}

start = async () => {
start = async (expectedBuilds = 1) => {
if (this.#watcher) {
throw new Error(`.start() may only be called once`);
}

this.#expectedBuilds = expectedBuilds;

let buildDirectory = this.#addon.dir;

/**
Expand All @@ -39,6 +45,13 @@ export class DevWatcher {
options.watch = {
buildDelay: 20,
};
// for local debugging
(options.plugins as Plugin[]).push({
name: 'watch change',
watchChange(id: string) {
console.log('changed:', id);
},
});
return options;
})
);
Expand All @@ -51,13 +64,18 @@ export class DevWatcher {
this.#watcher.on('event', args => {
switch (args.code) {
case 'START': {
this.#defer();
console.log('start');
break;
}
case 'END': {
console.log('end');
this.#forceResolve?.('end');
break;
}
case 'BUNDLE_END': {
args.result.close();
break;
}
case 'ERROR': {
this.#forceReject?.(args.error);
break;
Expand All @@ -67,42 +85,51 @@ export class DevWatcher {

this.#watcher.on('close', () => this.#forceResolve?.('close'));

return this.settled();
return this.nextBuild();
};

#defer = () => {
if (this.#waitForBuildPromise) {
// Need to finish prior work before deferring again
// if we hit this use case, we may have mis-configured
// the previosu deferral
// the previous deferral
return;
}
this.#counter = this.#expectedBuilds;
this.#waitForBuildPromise = new Promise<unknown>((_resolve, _reject) => {
this.#resolve = _resolve;
this.#resolve = (...args) => {
this.#counter -= 1;
if (this.#counter === 0) {
_resolve(...args);
}
};
this.#reject = _reject;
});
};

#forceResolve(state: unknown) {
this.#resolve?.(state);
this.#waitForBuildPromise = undefined;
}
#forceReject(error: unknown) {
this.#reject?.(error);
this.#waitForBuildPromise = undefined;
}

stop = async () => {
await this.#watcher?.close();
process.chdir(this.#originalDirectory);
};

nextBuild = async () => {
nextBuild = async (expectedBuilds = 1) => {
this.#expectedBuilds = expectedBuilds;
this.#defer();
await this.settled();
try {
await this.settled();
} finally {
this.#waitForBuildPromise = undefined;
}
};

settled = async (timeout = 5_000) => {
settled = async (timeout = 8_000) => {
if (!this.#waitForBuildPromise) {
console.debug(`There is nothing to wait for`);
return;
Expand Down
67 changes: 9 additions & 58 deletions tests/scenarios/v2-addon-dev-watch-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ Scenarios.fromProject(() => baseV2Addon())
);

await fs.rm(srcPathOther);
await watcher?.nextBuild();
// we expect 2 builds because appReexports plugin modifies package.json triggering a new build
await watcher?.nextBuild(2);
assert.strictEqual(
existsSync(distAppReExportPathOther),
false,
Expand All @@ -194,7 +195,8 @@ Scenarios.fromProject(() => baseV2Addon())

test('create a component in src should create it in dist', async function (assert) {
await fs.writeFile(srcPathOther, origContent);
await watcher?.nextBuild();
// we expect 2 builds because appReexports plugin modifies package.json triggering a new build
await watcher?.nextBuild(2);
assert.strictEqual(
existsSync(distAppReExportPathOther),
true,
Expand All @@ -206,16 +208,9 @@ Scenarios.fromProject(() => baseV2Addon())
await becomesModified({
filePath: distPathDemoComp,
assert,
// Update a component
fn: async () => {
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`);
await aBit(10);
await watcher?.nextBuild();
},
});
Expand All @@ -225,36 +220,23 @@ Scenarios.fromProject(() => baseV2Addon())
await becomesModified({
filePath: distPathDemoComp,
assert,
// Update a component
fn: async () => {
// 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.rm(demoHbs);
await watcher?.nextBuild();
},
});
});

test('updating hbs content should not update unrelated files', async function (assert) {
test('creating hbs content should not update unrelated files', async function (assert) {
await fs.writeFile(demoHbs, demoContent);
await watcher?.nextBuild();

await isNotModified({
filePath: distPath,
assert,
// Update a component
fn: async () => {
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 watcher?.nextBuild();
},
});
Expand All @@ -265,16 +247,9 @@ Scenarios.fromProject(() => baseV2Addon())
await isNotModified({
filePath: distPath,
assert,
// Update a component
fn: async () => {
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`);
await aBit(10);
await watcher?.nextBuild();
},
});
Expand All @@ -284,32 +259,22 @@ Scenarios.fromProject(() => baseV2Addon())
await becomesModified({
filePath: distPathOther,
assert,
// Update a component
fn: async () => {
await aBit(100);
let someContent = await fs.readFile(srcPathOther);

// 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(srcPathOther, someContent + `test\n`);
await aBit(10);
await watcher?.nextBuild();
},
});
});

test('deleting demo.js should make demo a template only component', async function (assert) {
await aBit(100);
await fs.rm(demoJs);
await watcher?.nextBuild();
let distPathDemoCompContent = await fs.readFile(distPathDemoComp);
assert.true(distPathDemoCompContent.includes('templateOnly'));
});

test('creating demo.js should make demo a template colocated component', async function (assert) {
await aBit(100);
void fs.writeFile(demoJs, demoJsContent);
await watcher?.nextBuild();
let distPathDemoCompContent = await fs.readFile(distPathDemoComp);
Expand All @@ -332,11 +297,6 @@ Scenarios.fromProject(() => baseV2Addon())
fn: async () => {
let someContent = await fs.readFile(someFile);

// 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 watcher?.nextBuild();
},
Expand All @@ -354,16 +314,11 @@ Scenarios.fromProject(() => baseV2Addon())
assert,
// Remove a component
fn: async () => {
// 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 Promise.all([
fs.rm(path.join(addon.dir, 'src/components/demo.js')),
fs.rm(path.join(addon.dir, 'src/components/demo.hbs')),
]);
await fs.rm(path.join(addon.dir, 'src/components/demo.js'));
await watcher?.nextBuild();
await fs.rm(path.join(addon.dir, 'src/components/demo.hbs'));
// we expect 2 builds because appReexports plugin modifies package.json triggering a new build
await watcher?.nextBuild(2);
},
});
});
Expand All @@ -386,7 +341,3 @@ Scenarios.fromProject(() => baseV2Addon())
});
});
});

function aBit(ms: number) {
return new Promise(resolve => setTimeout(resolve, ms));
}

0 comments on commit d0f9bc0

Please sign in to comment.