Skip to content

Commit

Permalink
feat(node): improve @nrwl/node:node executor to handle build process …
Browse files Browse the repository at this point in the history
…correctly in watch and no-watch mode (#9180)

* Build only triggered once on file change
* Kill previous process if it exists
* Wait for build process to finish before exiting
  • Loading branch information
jaysoo committed Mar 4, 2022
1 parent fa4cb1a commit 7084d19
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
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 @@ -95,6 +95,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 @@ -104,6 +106,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

0 comments on commit 7084d19

Please sign in to comment.