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

Makes stopWatch() async and graceful. #3361

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Makes stopWatch() async and graceful. #3361

merged 1 commit into from
Jul 11, 2024

Conversation

VividVisions
Copy link
Contributor

This is the counterpart to 11ty/eleventy-dev-server#85.

@zachleat zachleat added this to the Eleventy 3.0.0 milestone Jul 11, 2024
@zachleat zachleat merged commit d72c41e into 11ty:main Jul 11, 2024
@zachleat
Copy link
Member

Shipping with 3.0.0-alpha.17 Thank you!!

@zachleat
Copy link
Member

zachleat commented Jul 12, 2024

FYI I did need to revert the process.exit() removal here. ⌘+C wasn’t stopping the process. I moved it out of the stopWatch method though, so hopefully that will still satisfy your requirements

zachleat added a commit that referenced this pull request Jul 12, 2024
zachleat added a commit that referenced this pull request Jul 12, 2024
@VividVisions
Copy link
Contributor Author

Weird. All tests I did were successful. Did you try with the PR in eleventy-dev-server? Current server isn't async.

@zachleat
Copy link
Member

zachleat commented Jul 12, 2024

This isn’t a currently tested scenario, unfortunately. Can you check the changes I made linked above and see if they still align? I don’t expect that we’ll need to revert any of the async changes you made, it was only the process.exit() placement

@VividVisions
Copy link
Contributor Author

I'm sorry, my description of this PR should have been more thorough.

This PR goes hand in hand with my PR in eleventy-dev-server (11ty/eleventy-dev-server#85). They need each other to work properly. Applying only one but not the other doesn't improve anything, unfortunately.

Regarding the process not stopping, my best guess is: The current dev-server doesn't close the WebSocket connection(s), which would prevent a clean exit. Reintroducing process.exit() actually renders both PRs moot since it prevents a graceful shutdown. :(

Please let me know if I can support you in any way.

@zachleat
Copy link
Member

Restored your original changes and they’ll ship with Dev Server v2.0.2 in 3.0.0-alpha.18—thank you so much!

@VividVisions
Copy link
Contributor Author

That's awesome! Thank you!

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

Successfully merging this pull request may close these issues.

2 participants