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

fix(snowpack): exit process after success #956

Merged
merged 1 commit into from
Aug 28, 2020

Conversation

lukeed
Copy link
Contributor

@lukeed lukeed commented Aug 28, 2020

Changes

Ensures that the primary node process is exited, which cleans up any associated child processes spawned by it. Without this, build (specifically) can hang indefinitely even though it was successful and Snowpack knows it's complete.

Screenshots

There's some additional logging in here -- remnants of my hunt to make sure it wasn't a fault of Snowpack elsewhere. For example, the stopping is at the cleanup stage of the esbuild plugin.

Before

Snowpack prints "Build Complete" but the process never closes.

Screen Shot 2020-08-28 at 11 23 17 AM

After

The process exits successfully

Screen Shot 2020-08-28 at 11 25 30 AM

Testing

Did not add tests since nothing should be altered by default. The return already implied a 0 exit code, preventing the final process.exit(1) from running. By adding process.exit(0) we're basically just redefining the defaults.

Ensures that the primary node process is exited, which cleans up any associated child processes spawned by it. Without this, `build` (specifically) can hang indefinitely even though it was successful and Snowpack knows it's complete.
@vercel
Copy link

vercel bot commented Aug 28, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/d6j4mmc10
✅ Preview: https://snowpack-git-fork-lukeed-patch-1.pikapkg.vercel.app

@@ -105,15 +105,15 @@ export async function cli(args: string[]) {

if (cmd === 'build') {
await buildCommand(commandOptions);
return;
return process.exit(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the one that matters (and has likelihood of spawning subprocesses) -- the others were added for consistency.

@FredKSchott
Copy link
Owner

Curious: what specifically was causing it to hang for you?

@lukeed
Copy link
Contributor Author

lukeed commented Aug 28, 2020

I can go back and find out more granularly, but i noticed it after attaching svelte-preprocess for Svelte + TypeScript support.

@FredKSchott
Copy link
Owner

interesting!

@FredKSchott FredKSchott merged commit de040f2 into FredKSchott:master Aug 28, 2020
@lukeed lukeed deleted the patch-1 branch August 28, 2020 21:54
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.

2 participants