Skip to content

Commit

Permalink
fix(js): node executor should always log error
Browse files Browse the repository at this point in the history
  • Loading branch information
mandarini committed Jun 15, 2023
1 parent 7e85063 commit 3fd6ec6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 7 deletions.
49 changes: 49 additions & 0 deletions e2e/js/src/js-node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {
cleanupProject,
newProject,
runCLI,
uniq,
updateFile,
updateProjectConfig,
} from '@nx/e2e/utils';

describe('js:node error handling', () => {
let scope: string;

beforeEach(() => {
scope = newProject();
});

afterEach(() => cleanupProject());

it('should log out the error', () => {
const esbuildLib = uniq('esbuildlib');

runCLI(
`generate @nx/js:lib ${esbuildLib} --bundler=esbuild --no-interactive`
);

updateFile(`libs/${esbuildLib}/src/index.ts`, () => {
return `
console.log('Hello from my library!');
throw new Error('This is an error');
`;
});

updateProjectConfig(esbuildLib, (config) => {
config.targets['run-node'] = {
executor: '@nx/js:node',
options: {
buildTarget: `${esbuildLib}:build`,
watch: false,
},
};
return config;
});

const output = runCLI(`run ${esbuildLib}:run-node`);

expect(output).toContain('Hello from my library!');
expect(output).toContain('This is an error');
}, 240_000);
});
21 changes: 14 additions & 7 deletions packages/js/src/executors/node/node.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import { join } from 'path';

import { InspectType, NodeExecutorOptions } from './schema';
import { createAsyncIterable } from '@nx/devkit/src/utils/async-iterable';
import { calculateProjectDependencies } from '../../utils/buildable-libs-utils';
import {
calculateProjectDependencies,
getOutputFileName,
} from '../../utils/buildable-libs-utils';
import { killTree } from './lib/kill-tree';

interface ActiveTask {
Expand Down Expand Up @@ -71,11 +74,14 @@ export async function* nodeExecutor(

// Re-map buildable workspace projects to their output directory.
const mappings = calculateResolveMappings(context, options);
const fileToRun = join(
context.root,
buildOptions.outputPath,
buildOptions.outputFileName ?? 'main.js'
);

let outputFileName = buildOptions.outputFileName;

if (!outputFileName) {
outputFileName = getOutputFileName(buildOptions.main);
}

let fileToRun = join(context.root, buildOptions.outputPath, outputFileName);

const tasks: ActiveTask[] = [];
let currentTask: ActiveTask = null;
Expand Down Expand Up @@ -168,7 +174,8 @@ export async function* nodeExecutor(
task.childProcess.stderr.on('data', (data) => {
// Don't log out error if task is killed and new one has started.
// This could happen if a new build is triggered while new process is starting, since the operation is not atomic.
if (options.watch && !task.killed) {
// Log the error in normal mode
if (!options.watch || !task.killed) {
logger.error(data.toString());
}
});
Expand Down
9 changes: 9 additions & 0 deletions packages/js/src/utils/buildable-libs-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,12 @@ function hasDependency(outputJson, depConfigName: string, packageName: string) {
return false;
}
}

export function getOutputFileName(path: string): string {
const filename: string = path.split('/').pop() || '';
const filenameWithoutExtension: string = filename
.split('.')
.slice(0, -1)
.join('.');
return `${filenameWithoutExtension}.js`;
}

0 comments on commit 3fd6ec6

Please sign in to comment.