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.

Sign up for GitHub

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

Jump to bottom

fix(js): node executor should always log error #17622

Merged
merged 1 commit into from
Jun 16, 2023
Merged

fix(js): node executor should always log error #17622

merged 1 commit into from
Jun 16, 2023

Conversation

mandarini
Copy link
Member

@mandarini mandarini commented Jun 15, 2023 •
edited
Loading

Current Behavior

When using the nx/js:node executor, error is swallowed unless you're in watch mode.

Expected Behavior

Error should always be logged.

Also, we should infer the outputFileName from the main entrypoint as set in build options.

Related Issue(s)

Fixes #17595 #17627 #17083

@mandarini mandarini requested a review from a team as a code owner June 15, 2023 17:05
@mandarini mandarini requested a review from ndcunningham June 15, 2023 17:05
@vercel
Copy link

vercel bot commented Jun 15, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 16, 2023 3:58pm

@nx-cloud
Copy link

nx-cloud bot commented Jun 15, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1f6011c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 8 targets

Sent with 💌 from NxCloud.

@mandarini mandarini force-pushed the fix/js-node-throw-error branch from ba6a2cc to ef3670e Compare June 15, 2023 17:18
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from ef3670e to 8af0851 Compare June 15, 2023 17:22
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 8af0851 to 3fd6ec6 Compare June 15, 2023 17:25
@mandarini mandarini force-pushed the fix/js-node-throw-error branch 2 times, most recently from 3bfc7e9 to 0198de4 Compare June 15, 2023 17:38
@mandarini mandarini requested a review from jaysoo June 15, 2023 17:44
@mandarini mandarini self-assigned this Jun 15, 2023
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 0198de4 to dbf8505 Compare June 16, 2023 09:38
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from dbf8505 to e74c18b Compare June 16, 2023 10:08
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from cb63166 to e05bf5a Compare June 16, 2023 12:16
@mandarini mandarini requested review from meeroslav, a team and vsavkin as code owners June 16, 2023 12:16
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from e05bf5a to 7f7a815 Compare June 16, 2023 12:18
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 7f7a815 to 179893e Compare June 16, 2023 12:25
meeroslav
meeroslav approved these changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@meeroslav meeroslav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 179893e to 4a6c86c Compare June 16, 2023 13:47
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 4a6c86c to 75c980b Compare June 16, 2023 13:56
mandarini
mandarini commented Jun 16, 2023
View reviewed changes
e2e/utils/command-utils.ts Outdated Show resolved Hide resolved
This was linked to issues Jun 16, 2023
js: doesn't show any errors from stderr #17627
Closed
js: silently fails to run a file #17083
Closed
@mandarini mandarini enabled auto-merge (squash) June 16, 2023 14:02
@mandarini mandarini force-pushed the fix/js-node-throw-error branch 2 times, most recently from 541b33c to 5c6248c Compare June 16, 2023 15:16
@mandarini mandarini force-pushed the fix/js-node-throw-error branch from 5c6248c to 1f6011c Compare June 16, 2023 15:55
@mandarini mandarini merged commit 36838d6 into nrwl:master Jun 16, 2023
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 22, 2023
@mandarini mandarini deleted the fix/js-node-throw-error branch August 22, 2023 12:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@meeroslav meeroslav meeroslav approved these changes

@ndcunningham ndcunningham ndcunningham approved these changes

@jaysoo jaysoo jaysoo approved these changes

@vsavkin vsavkin Awaiting requested review from vsavkin vsavkin is a code owner

Assignees

@mandarini mandarini

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@mandarini @jaysoo @ndcunningham @meeroslav