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.

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

Use worker threads when running lint --serially #607

Merged
merged 3 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/metal-rings-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"skuba": patch
---

lint: Use worker threads when running `--serial`ly

This aims to reduce the memory footprint of `skuba lint --serial`. ESLint and Prettier are now run in worker threads so their memory can be more readily freed on thread exit.
4 changes: 2 additions & 2 deletions src/cli/lint.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ test.each`
await prepareTempDirectory(baseDir, tempDir);

await expect(
// Force `--serial` as worker threads can't run in a TypeScript context.
lint([...args, '--serial'], tscOutputStream),
// Disable worker threads as they can't run in a TypeScript context.
lint(args, tscOutputStream, false),
).resolves.toBeUndefined();

const tempDirRegex = new RegExp(tempDir, 'g');
Expand Down
29 changes: 26 additions & 3 deletions src/cli/lint/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,42 @@ const lintConcurrently = async ({ tscOutputStream, ...input }: Input) => {
return { eslint, prettier, tscOk };
};

const lintSerially = async (input: Input) => {
/**
* Run linting tools `--serial`ly for resource-constrained environments.
*
* Note that we still run ESLint and Prettier in worker threads as a
* counterintuitive optimisation. Memory can be more readily freed on worker
* thread exit, which isn't as easy with a monolithic main thread.
*/
const lintSerially = async ({ tscOutputStream, ...input }: Input) => {
const eslint = await runESLintInWorkerThread(input);
const prettier = await runPrettierInWorkerThread(input);
const tscOk = await runTscInNewProcess({ ...input, tscOutputStream });

return { eslint, prettier, tscOk };
};

const lintSeriallyWithoutWorkerThreads = async (input: Input) => {
const eslint = await runESLintInCurrentThread(input);
const prettier = await runPrettierInCurrentThread(input);
const tscOk = await runTscInNewProcess(input);

return { eslint, prettier, tscOk };
};

export const externalLint = async (input: Input) => {
const selectLintFunction = (input: Input) => {
if (!input.workerThreads) {
return lintSeriallyWithoutWorkerThreads;
}

// `--debug` implies `--serial`.
const isSerial = input.debug || input.serial;

const lint = isSerial ? lintSerially : lintConcurrently;
return isSerial ? lintSerially : lintConcurrently;
};

export const externalLint = async (input: Input) => {
const lint = selectLintFunction(input);

const tscOutputStream = new StreamInterceptor();
tscOutputStream.pipe(input.tscOutputStream ?? process.stdout);
Expand Down
2 changes: 2 additions & 0 deletions src/cli/lint/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import type { Input } from './types';
export const lint = async (
args = process.argv,
tscOutputStream: NodeJS.WritableStream | undefined = undefined,
workerThreads = true,
) => {
const opts: Input = {
debug: hasDebugFlag(args),
serial: hasSerialFlag(args),
tscOutputStream,
workerThreads,
};

await externalLint(opts);
Expand Down
10 changes: 10 additions & 0 deletions src/cli/lint/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,14 @@ export interface Input {
* Defaults to `process.stdout`.
*/
tscOutputStream?: NodeJS.WritableStream;

/**
* Whether to allow usage of Node.js worker threads.
*
* This may be set to `false` when there is a worker thread incompatibility,
* such as calling in from a TypeScript context in our Jest tests.
*
* Defaults to `true`.
*/
workerThreads?: boolean;
}