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

Do not drop aggregated changes with resumed compilation #12881

Closed
wants to merge 6 commits into from

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Mar 12, 2021

Description

This PR aims to fix an issue with suspend and resume flow of webpack Watching (see #12882). The root cause of the issue is that watchpack instance is closed before aggregated event is emitted (but after the change event). So aggregated changes are silently dropped.

I've added the simplest test to demonstrate this but there are other code paths where the same underlying problem occurs.

The fix I've added so far is just a draft to demonstrate a possible solution. But ideally, we should patch watchpack to allow flushing pending aggregated events when closing the watcher.

Can open a PR in watchpack with required changes. But first, want to make sure it is a viable solution.

What kind of change does this PR introduce?

A bugfix

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

An anticipated fix should be backwards compatible.

What needs to be documented once your changes are merged?

This PR doesn't require documentation.

Fixes #12882

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@vladar vladar marked this pull request as ready for review March 13, 2021 13:37
@vladar vladar marked this pull request as draft March 13, 2021 19:18
@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@vladar vladar marked this pull request as ready for review March 17, 2021 10:16
@tu4mo
Copy link

tu4mo commented Apr 21, 2021

Any chance getting this reviewed soon?

sokra added a commit that referenced this pull request May 18, 2021
Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com>
@sokra
Copy link
Member

sokra commented May 19, 2021

Merged by #13399

@sokra sokra closed this May 19, 2021
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.

Webpack drops aggregated changes when using Watching.resume()
4 participants