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

Ensure errors are emitted when ts:precompile fails #984

Merged
merged 5 commits into from
Dec 5, 2019

Conversation

dfreeman
Copy link
Member

@dfreeman dfreeman commented Dec 2, 2019

As noted both in Discord and in #973, we currently don't emit any of tsc's output when it fails during ts:precompile. This ensures we capture and re-emit any output when the command exits with a nonzero code, and adds a test to make sure we don't regress in the future.

@dfreeman
Copy link
Member Author

dfreeman commented Dec 2, 2019

Floating-deps tests should start passing again once zloirock/core-js#710 is resolved

@chriskrycho
Copy link
Member

Updated from master, rerunning CI now. Also reviewing!

'--pretty', 'true',
], {
preferLocal: true,
all: true,
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding a comment here explaining why we want this? Will make maintenance easier. (I just went and read the execa docs to figure it out, figure it'll save us time in the future from doing the same.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! execa is one of those that I've used enough places that I've lost any good sense of which options are self-describing and which aren't 🙃

Copy link
Member

Choose a reason for hiding this comment

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

I sympathize!

@chriskrycho chriskrycho merged commit 54cdb8d into master Dec 5, 2019
@delete-merged-branch delete-merged-branch bot deleted the emit-precompile-errors branch December 5, 2019 23:33
@ef4
Copy link
Collaborator

ef4 commented Dec 9, 2019

Thanks, I just hit this bug and was about to make the same PR when I saw it's already fixed on master.

@chriskrycho chriskrycho mentioned this pull request Dec 13, 2019
@jamescdavis
Copy link
Member

Released in 3.1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants