From 143497d4d6881dd7996ebc76da4a5e950b12558d Mon Sep 17 00:00:00 2001 From: Jason Gao Date: Thu, 18 May 2023 14:55:51 -0400 Subject: [PATCH 1/2] child process should ignore files that are specified in the ignore array --- spec/Supervisor.spec.ts | 64 ++++++++++++------------------- src/Supervisor.ts | 1 + src/SwcCompiler.ts | 33 ++++++++++------ src/bench/scripts/ignored.ts | 1 + src/child-process-registration.ts | 16 ++++++-- src/index.ts | 10 ++++- wds.js | 3 ++ 7 files changed, 72 insertions(+), 56 deletions(-) create mode 100644 src/bench/scripts/ignored.ts create mode 100644 wds.js diff --git a/spec/Supervisor.spec.ts b/spec/Supervisor.spec.ts index 82a5aaa..4062459 100644 --- a/spec/Supervisor.spec.ts +++ b/spec/Supervisor.spec.ts @@ -1,6 +1,6 @@ import { ChildProcess, spawn } from "child_process"; -import * as path from "path"; import { range } from "lodash"; +import * as path from "path"; const childExit = (child: ChildProcess) => { return new Promise((resolve) => { @@ -9,28 +9,24 @@ const childExit = (child: ChildProcess) => { expect(code).toEqual(0); }); }); -} +}; test("it proxies ipc messages", async () => { const binPath = path.join(__dirname, "../pkg/wds.bin.js"); const scriptPath = path.join(__dirname, "fixtures/src/add.ts"); - const child = spawn( - "node", - [binPath, scriptPath], - { - stdio: ["inherit", "inherit", "inherit", "ipc"], - env: process.env, - } - ); + const child = spawn("node", [binPath, scriptPath], { + stdio: ["inherit", "inherit", "inherit", "ipc"], + env: process.env, + }); const childHasBooted = new Promise((resolve) => { const handler = () => { resolve(); - child.off("message", handler) + child.off("message", handler); }; - child.on("message", handler) - }) + child.on("message", handler); + }); await childHasBooted; const messagesToChild = range(0, 3); @@ -38,7 +34,7 @@ test("it proxies ipc messages", async () => { const promise = new Promise((resolve) => { child.on("message", (message: any) => { - messagesFromChild.push(message) + messagesFromChild.push(message); if (messagesFromChild.length === messagesToChild.length) { resolve(); @@ -55,20 +51,16 @@ test("it proxies ipc messages", async () => { await promise; expect(messagesFromChild).toEqual([1, 2, 3]); -}) +}); test("it doesn't setup ipc if it wasn't setup with ipc itself", async () => { const binPath = path.join(__dirname, "../pkg/wds.bin.js"); const scriptPath = path.join(__dirname, "fixtures/src/no-ipc.ts"); - const child = spawn( - "node", - [binPath, scriptPath], - { - stdio: ["inherit", "inherit", "inherit"], - env: process.env, - } - ); + const child = spawn("node", [binPath, scriptPath], { + stdio: ["inherit", "inherit", "inherit"], + env: process.env, + }); await childExit(child); }); @@ -77,13 +69,9 @@ test("it inherits stdin if WDS was started without terminal commands", async () const binPath = path.join(__dirname, "../pkg/wds.bin.js"); const scriptPath = path.join(__dirname, "fixtures/src/echo.ts"); - const child = spawn( - "node", - [binPath, scriptPath], - { - env: process.env, - } - ); + const child = spawn("node", [binPath, scriptPath], { + env: process.env, + }); let output = ""; @@ -92,7 +80,7 @@ test("it inherits stdin if WDS was started without terminal commands", async () child.stdout.on("data", (data) => { output += data; - }) + }); await childExit(child); expect(output).toEqual("test"); @@ -102,13 +90,9 @@ test("it doesn't have any stdin if wds is started with terminal commands", async const binPath = path.join(__dirname, "../pkg/wds.bin.js"); const scriptPath = path.join(__dirname, "fixtures/src/echo.ts"); - const child = spawn( - "node", - [binPath, scriptPath, "--commands"], - { - env: process.env, - } - ); + const child = spawn("node", [binPath, scriptPath, "--commands"], { + env: process.env, + }); let output = ""; @@ -117,9 +101,9 @@ test("it doesn't have any stdin if wds is started with terminal commands", async child.stdout.on("data", (data) => { output += data; - }) + }); await childExit(child); expect(output).toEqual(""); -}) +}); diff --git a/src/Supervisor.ts b/src/Supervisor.ts index 4181ab8..b69c0c3 100644 --- a/src/Supervisor.ts +++ b/src/Supervisor.ts @@ -49,6 +49,7 @@ export class Supervisor extends EventEmitter { ...process.env, WDS_SOCKET_PATH: this.socketPath, WDS_EXTENSIONS: this.project.config.extensions.join(","), + WDS_IGNORE: JSON.stringify(this.project.config.ignore), }, stdio: stdio, }); diff --git a/src/SwcCompiler.ts b/src/SwcCompiler.ts index f0489ea..3006a73 100644 --- a/src/SwcCompiler.ts +++ b/src/SwcCompiler.ts @@ -11,7 +11,14 @@ import { log, projectConfig } from "./utils"; // https://esbuild.github.io/api/#resolve-extensions const DefaultExtensions = [".tsx", ".ts", ".jsx", ".mjs", ".cjs", ".js"]; -export class MissingDestinationError extends Error {} +export class MissingDestinationError extends Error { + ignoredFile: boolean; + + constructor(error: { message: string; ignoredFile?: boolean }) { + super(error.message); + this.ignoredFile = !!error.ignoredFile; + } +} const SWC_DEFAULTS: Config = { env: { @@ -178,9 +185,7 @@ export class SwcCompiler implements Compiler { // TODO: Use the config const { root, fileNames, swcConfig } = await this.getModule(filename); - await this.reportErrors(async () => { - await Promise.all(fileNames.map((filename) => this.buildFile(filename, root, swcConfig))); - }); + await this.reportErrors(Promise.allSettled(fileNames.map((filename) => this.buildFile(filename, root, swcConfig)))); log.debug("started build", { root, @@ -190,11 +195,11 @@ export class SwcCompiler implements Compiler { }); } - private async reportErrors(run: () => Promise) { - try { - return await run(); - } catch (error) { - log.error(error); + private async reportErrors(results: Promise[]>) { + for (const result of await results) { + if (result.status === "rejected") { + log.error(result.reason); + } } } @@ -203,9 +208,15 @@ export class SwcCompiler implements Compiler { // TODO: Understand cases in which the file destination could be missing if (ignorePattern) { - return `File ${filename} is imported but not being built because it is explicitly ignored in the wds project config. It is being ignored by the provided glob pattern '${ignorePattern}', remove this pattern from the project config or don't import this file to fix.`; + return { + message: `File ${filename} is imported but not being built because it is explicitly ignored in the wds project config. It is being ignored by the provided glob pattern '${ignorePattern}', remove this pattern from the project config or don't import this file to fix.`, + ignoredFile: true, + }; } else { - return `Built output for file ${filename} not found. Is it outside the project directory, or has it failed to build?`; + return { + message: `Built output for file ${filename} not found. Is it outside the project directory, or has it failed to build?`, + ignoredFile: false, + }; } } diff --git a/src/bench/scripts/ignored.ts b/src/bench/scripts/ignored.ts new file mode 100644 index 0000000..10e9875 --- /dev/null +++ b/src/bench/scripts/ignored.ts @@ -0,0 +1 @@ +// this is ignored diff --git a/src/child-process-registration.ts b/src/child-process-registration.ts index be8b843..69b3d45 100644 --- a/src/child-process-registration.ts +++ b/src/child-process-registration.ts @@ -36,7 +36,13 @@ const notifyParentProcessOfRequire = (filename: string) => { if (!workerData || !(workerData as SyncWorkerData).isWDSSyncWorker) { const worker = new SyncWorker(path.join(__dirname, "child-process-ipc-worker.js")); - const paths: Record = {}; + const paths: Record< + string, + | string + | { + ignored: boolean; + } + > = {}; // Compile a given file by sending it into our async-to-sync wrapper worker js file // The leader process returns us a list of all the files it just compiled, so that we don't have to pay the IPC boundary cost for each file after this one @@ -63,9 +69,11 @@ if (!workerData || !(workerData as SyncWorkerData).isWDSSyncWorker) { for (const extension of process.env["WDS_EXTENSIONS"]!.split(",")) { require.extensions[extension] = (module: any, filename: string) => { const compiledFilename = compile(filename); - const content = fs.readFileSync(compiledFilename, "utf8"); - notifyParentProcessOfRequire(filename); - module._compile(content, filename); + if (typeof compiledFilename === "string" || !compiledFilename.ignored) { + const content = fs.readFileSync(compiledFilename, "utf8"); + notifyParentProcessOfRequire(filename); + module._compile(content, filename); + } }; } } diff --git a/src/index.ts b/src/index.ts index d82e762..2559ed1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,7 +10,7 @@ import { hideBin } from "yargs/helpers"; import type { RunOptions } from "./Options"; import { Project } from "./Project"; import { Supervisor } from "./Supervisor"; -import { SwcCompiler } from "./SwcCompiler"; +import { MissingDestinationError, SwcCompiler } from "./SwcCompiler"; import { MiniServer } from "./mini-server"; import { log, projectConfig } from "./utils"; @@ -93,6 +93,14 @@ const startIPCServer = async (socketPath: string, project: Project) => { return await project.compiler.fileGroup(filename); } catch (error) { log.error(`Error compiling file ${filename}:`, error); + + if (error instanceof MissingDestinationError && error.ignoredFile) { + return { + [filename]: { + ignored: true, + }, + }; + } } }; diff --git a/wds.js b/wds.js new file mode 100644 index 0000000..0906dcd --- /dev/null +++ b/wds.js @@ -0,0 +1,3 @@ +module.exports = { + ignore: ["src/bench/scripts/ignored.ts"], + }; \ No newline at end of file From 5b30ac2a80de3dbac579dd294d18add0054e433b Mon Sep 17 00:00:00 2001 From: Jason Gao Date: Fri, 19 May 2023 12:46:09 -0400 Subject: [PATCH 2/2] add tests to ensure ignored files are ignored by the child process as well --- spec/Supervisor.spec.ts | 2 +- spec/SwcCompiler.test.ts | 62 +++++++++++-- spec/fixtures/failing/bar.ts | 1 + spec/fixtures/failing/successful.ts | 3 + .../src/files_with_config}/ignored.ts | 0 spec/fixtures/src/files_with_config/wds.js | 3 +- spec/fixtures/src/wds.js | 2 +- spec/wds.spec.ts | 90 +++++++++++++++++++ src/Supervisor.ts | 1 - src/index.ts | 4 +- tsconfig.json | 2 +- wds.js | 3 - 12 files changed, 156 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/failing/bar.ts create mode 100644 spec/fixtures/failing/successful.ts rename {src/bench/scripts => spec/fixtures/src/files_with_config}/ignored.ts (100%) create mode 100644 spec/wds.spec.ts delete mode 100644 wds.js diff --git a/spec/Supervisor.spec.ts b/spec/Supervisor.spec.ts index 4062459..20debe9 100644 --- a/spec/Supervisor.spec.ts +++ b/spec/Supervisor.spec.ts @@ -42,7 +42,7 @@ test("it proxies ipc messages", async () => { }); }); - for (let number of messagesToChild) { + for (const number of messagesToChild) { child.send(number); } diff --git a/spec/SwcCompiler.test.ts b/spec/SwcCompiler.test.ts index 2079b68..6055525 100644 --- a/spec/SwcCompiler.test.ts +++ b/spec/SwcCompiler.test.ts @@ -1,40 +1,86 @@ -import { MissingDestinationError, SwcCompiler } from '../src/SwcCompiler' import * as fs from "fs/promises"; import os from "os"; import path from "path"; +import { MissingDestinationError, SwcCompiler } from "../src/SwcCompiler"; +import { log } from "../src/utils"; const compile = async (filename: string, root = "fixtures/src") => { const workDir = await fs.mkdtemp(path.join(os.tmpdir(), "wds-test")); const rootDir = path.join(__dirname, root); const fullPath = path.join(rootDir, filename); - const compiler = new SwcCompiler(rootDir, workDir) + const compiler = new SwcCompiler(rootDir, workDir); await compiler.compile(fullPath); const compiledFilePath = (await compiler.fileGroup(fullPath))[fullPath]!; return await fs.readFile(compiledFilePath, "utf-8"); -} +}; test("compiles simple files", async () => { const content = await compile("./simple.ts"); - expect(content).toContain('console.log("success")') + expect(content).toContain('console.log("success")'); }); test("throws if the compilation fails", async () => { await expect(compile("./failing/failing.ts", "fixtures/failing")).rejects.toThrow(MissingDestinationError); -}) +}); + +test("throws if the file is ignored", async () => { + let error: MissingDestinationError | null = null; + try { + await compile("./files_with_config/ignored.ts"); + } catch (e) { + if (e instanceof MissingDestinationError) { + error = e; + } else { + throw e; + } + } + + expect(error).toBeDefined(); + expect(error?.ignoredFile).toBeTruthy(); + expect(error?.message).toMatch( + /File .+ignored\.ts is imported but not being built because it is explicitly ignored in the wds project config\. It is being ignored by the provided glob pattern '!ignored\.ts', remove this pattern from the project config or don't import this file to fix./ + ); +}); + +test("logs error when a file in group fails compilation but continues", async () => { + const errorLogs: any[] = []; + + const mock = jest.spyOn(log, "error").mockImplementation((...args: any[]) => { + errorLogs.push(args); + }); + + const workDir = await fs.mkdtemp(path.join(os.tmpdir(), "wds-test")); + const rootDir = path.join(__dirname, "fixtures/failing"); + const fullPath = path.join(rootDir, "successful.ts"); + const compiler = new SwcCompiler(rootDir, workDir); + await compiler.compile(fullPath); + const group = await compiler.fileGroup(fullPath); + + expect(group[fullPath]).toBeDefined(); + expect(Object.entries(group).filter(([path]) => /.+(bar|successful)\.ts$/.test(path))).toHaveLength(2); + const error = errorLogs[0][0]; + expect(error.code).toBe("GenericFailure"); + expect(error.message).toMatch(/.+failing\.ts/); + expect(error.message).toMatch(/Syntax Error/); + + mock.mockRestore(); +}); test("compiles lazy import", async () => { const content = await compile("./lazy_import.ts"); - expect(content).toContain(` + expect(content).toContain( + ` function _child_process() { - const data = require(\"child_process\"); + const data = require("child_process"); _child_process = function() { return data; }; return data; } -`.trim()); +`.trim() + ); }); test("uses the swc config file from wds.js", async () => { diff --git a/spec/fixtures/failing/bar.ts b/spec/fixtures/failing/bar.ts new file mode 100644 index 0000000..61d366e --- /dev/null +++ b/spec/fixtures/failing/bar.ts @@ -0,0 +1 @@ +export const foo = "foo"; diff --git a/spec/fixtures/failing/successful.ts b/spec/fixtures/failing/successful.ts new file mode 100644 index 0000000..c65ae90 --- /dev/null +++ b/spec/fixtures/failing/successful.ts @@ -0,0 +1,3 @@ +export function success() { + return !!(1 + 1); +} diff --git a/src/bench/scripts/ignored.ts b/spec/fixtures/src/files_with_config/ignored.ts similarity index 100% rename from src/bench/scripts/ignored.ts rename to spec/fixtures/src/files_with_config/ignored.ts diff --git a/spec/fixtures/src/files_with_config/wds.js b/spec/fixtures/src/files_with_config/wds.js index 145d6c8..2f84ef1 100644 --- a/spec/fixtures/src/files_with_config/wds.js +++ b/spec/fixtures/src/files_with_config/wds.js @@ -10,5 +10,6 @@ module.exports = { "type": "commonjs", "strictMode": false } - } + }, + ignore: ["ignored.ts"] } diff --git a/spec/fixtures/src/wds.js b/spec/fixtures/src/wds.js index 9fbe59c..313aba9 100644 --- a/spec/fixtures/src/wds.js +++ b/spec/fixtures/src/wds.js @@ -13,5 +13,5 @@ module.exports = { "strictMode": true, "lazy": true } - } + }, } diff --git a/spec/wds.spec.ts b/spec/wds.spec.ts new file mode 100644 index 0000000..29904e3 --- /dev/null +++ b/spec/wds.spec.ts @@ -0,0 +1,90 @@ +import assert from "assert"; +import type { ChildProcess } from "child_process"; +import http from "http"; +import path from "path"; +import { Supervisor } from "../src/Supervisor"; +import { wds } from "../src/index"; + +describe("wds", () => { + let cwd: any; + let supervisorRestart: any; + let socketPath: string; + + const sendCompileRequest = async (filename: string) => { + assert(socketPath, "socketPath must be set"); + const result = await new Promise((resolve, reject) => { + const request = http.request({ socketPath, path: "/compile", method: "POST", timeout: 200 }, (resp) => { + let data = ""; + if (resp.statusCode !== 200) { + return reject(`Error compiling`); + } + resp.on("data", (chunk: string) => (data += chunk)); + resp.on("end", () => resolve(JSON.parse(data).filenames)); + }); + + request.on("error", (error) => { + reject(error); + }); + request.write(filename); + request.end(); + }); + + return result; + }; + + beforeEach(() => { + cwd = jest.spyOn(process, "cwd").mockImplementation(() => { + return path.resolve(__dirname, "fixtures/src/files_with_config"); + }); + + supervisorRestart = jest.spyOn(Supervisor.prototype, "restart").mockImplementation(function () { + const self = this as unknown as Supervisor; + socketPath = self.socketPath; + self.process = { + on: jest.fn(), + } as unknown as ChildProcess; + return self.process; + }); + }); + + afterEach(() => { + cwd.mockRestore(); + supervisorRestart.mockRestore(); + }); + + test("server responds to ignored files", async () => { + const server = await wds({ + argv: [], + terminalCommands: false, + reloadOnChanges: false, + }); + const result = (await sendCompileRequest(path.resolve(__dirname, "fixtures/src/files_with_config/ignored.ts"))) as Record< + string, + string | { ignored: boolean } + >; + const compiledKeys = Object.keys(result).filter((k) => /spec\/fixtures\/src\/files_with_config\/ignored\.ts/.test(k)); + expect(compiledKeys).toHaveLength(1); + expect(result[compiledKeys[0]]).toEqual({ + ignored: true, + }); + + server.close(); + }); + + test("server responds to included files", async () => { + const server = await wds({ + argv: [], + terminalCommands: false, + reloadOnChanges: false, + }); + const result = (await sendCompileRequest(path.resolve(__dirname, "fixtures/src/files_with_config/simple.ts"))) as Record< + string, + string | { ignored: boolean } + >; + const compiledKeys = Object.keys(result).filter((k) => /spec\/fixtures\/src\/files_with_config\/simple\.ts/.test(k)); + expect(compiledKeys).toHaveLength(1); + expect(typeof result[compiledKeys[0]]).toBe("string"); + + server.close(); + }); +}); diff --git a/src/Supervisor.ts b/src/Supervisor.ts index b69c0c3..4181ab8 100644 --- a/src/Supervisor.ts +++ b/src/Supervisor.ts @@ -49,7 +49,6 @@ export class Supervisor extends EventEmitter { ...process.env, WDS_SOCKET_PATH: this.socketPath, WDS_EXTENSIONS: this.project.config.extensions.join(","), - WDS_IGNORE: JSON.stringify(this.project.config.ignore), }, stdio: stdio, }); diff --git a/src/index.ts b/src/index.ts index 2559ed1..7b65996 100644 --- a/src/index.ts +++ b/src/index.ts @@ -148,7 +148,7 @@ export const wds = async (options: RunOptions) => { if (options.reloadOnChanges) startFilesystemWatcher(project); if (options.terminalCommands) startTerminalCommandListener(project); - await startIPCServer(serverSocketPath, project); + const server = await startIPCServer(serverSocketPath, project); // kickoff the first child process options.reloadOnChanges && log.info(`Supervision starting for command: node ${options.argv.join(" ")}`); @@ -174,4 +174,6 @@ export const wds = async (options: RunOptions) => { logShutdown("shutting down project since it's no longer needed..."); project.shutdown(code ?? 1); }); + + return server; }; diff --git a/tsconfig.json b/tsconfig.json index b0fbf3e..7901067 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -22,6 +22,6 @@ "node_modules/@types/jest/index.d.ts" ], "exclude": [ - "spec/fixtures", + "spec/fixtures" ] } diff --git a/wds.js b/wds.js deleted file mode 100644 index 0906dcd..0000000 --- a/wds.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - ignore: ["src/bench/scripts/ignored.ts"], - }; \ No newline at end of file