Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watch previous detected files in publicEntrypoints plugin #2208

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Watch previous detected files in publicEntrypoints plugin #2208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/addon-dev/src/rollup-app-reexports.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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));
}
Loading