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

feat(node): improve @nrwl/node:node executor to handle build process correctly in watch and no-watch mode #9180

Merged
merged 1 commit into from Mar 4, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 29 additions & 12 deletions packages/node/src/executors/node/node.impl.ts
Expand Up @@ -5,12 +5,12 @@ import {
parseTargetString,
runExecutor,
} from '@nrwl/devkit';
import { readCachedProjectGraph } from '@nrwl/workspace/src/core/project-graph';
import { calculateProjectDependencies } from '@nrwl/workspace/src/utilities/buildable-libs-utils';
import { ChildProcess, fork } from 'child_process';
import * as treeKill from 'tree-kill';
import { promisify } from 'util';
import { InspectType, NodeExecutorOptions } from './schema';
import { readCachedProjectGraph } from '@nrwl/workspace/src/core/project-graph';
import { calculateProjectDependencies } from '@nrwl/workspace/src/utilities/buildable-libs-utils';

let subProcess: ChildProcess = null;

Expand All @@ -24,15 +24,15 @@ export async function* nodeExecutor(
context: ExecutorContext
) {
process.on('SIGTERM', async () => {
await killProcess();
await killCurrentProcess();
process.exit(128 + 15);
});
process.on('SIGINT', async () => {
await killProcess();
await killCurrentProcess();
process.exit(128 + 2);
});
process.on('SIGHUP', async () => {
await killProcess();
await killCurrentProcess();
process.exit(128 + 1);
});

Expand Down Expand Up @@ -80,7 +80,7 @@ function calculateResolveMappings(
}, {});
}

function runProcess(
async function runProcess(
event: ExecutorEvent,
options: NodeExecutorOptions,
mappings: { [project: string]: string }
Expand All @@ -98,6 +98,18 @@ function runProcess(
},
}
);

if (!options.watch) {
return new Promise<void>((resolve, reject) => {
subProcess.on('exit', (code) => {
if (code === 0) {
resolve(undefined);
} else {
reject();
}
});
});
}
}

function getExecArgv(options: NodeExecutorOptions) {
Expand All @@ -123,17 +135,22 @@ async function handleBuildEvent(
options: NodeExecutorOptions,
mappings: { [project: string]: string }
) {
if ((!event.success || options.watch) && subProcess) {
await killProcess();
// Don't kill previous run unless new build is successful.
if (options.watch && event.success) {
await killCurrentProcess();
}

if (event.success) {
runProcess(event, options, mappings);
await runProcess(event, options, mappings);
}
}

async function killProcess() {
const promisifiedTreeKill: (pid: number, signal: string) => Promise<void> =
promisify(treeKill);
const promisifiedTreeKill: (pid: number, signal: string) => Promise<void> =
promisify(treeKill);

async function killCurrentProcess() {
if (!subProcess) return;

try {
await promisifiedTreeKill(subProcess.pid, 'SIGTERM');
} catch (err) {
Expand Down
5 changes: 5 additions & 0 deletions packages/node/src/utils/config.ts
Expand Up @@ -94,6 +94,8 @@ export function getBaseWebpackPartial(
},
plugins: [
new ForkTsCheckerWebpackPlugin({
// For watch mode, type errors should result in failure.
async: false,
typescript: {
enabled: true,
configFile: options.tsConfig,
Expand All @@ -103,6 +105,9 @@ export function getBaseWebpackPartial(
],
watch: options.watch,
watchOptions: {
// Delay the next rebuild from first file change, otherwise can lead to
// two builds on a single file change.
aggregateTimeout: 200,
poll: options.poll,
},
stats: getStatsConfig(options),
Expand Down