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: do not skip changes when compilation is suspended #12897

Closed
wants to merge 2 commits into from

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Mar 15, 2021

This PR aims to solve an issue with skipped changes in Watching.js (when compilation is suspended). See #12898 for details.

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?

No.

What needs to be documented once your changes are merged?

This PR doesn't require documentation.

Fixes #12898

@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 changed the title @vladar fix: do not skip changes when compilation is suspended fix: do not skip changes when compilation is suspended Mar 15, 2021
@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 15, 2021 19:18
@@ -266,7 +266,11 @@ class Watching {
this.watcher.pause();
Copy link
Member

Choose a reason for hiding this comment

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

I guess we should not pause the watcher when suspended, so it keeps aggregating changes. Otherwise modifiedFiles etc. do not contain the changes from the first watch event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am maybe missing something but it seems to be a dead code anyways, because this.watcher is always null at this point so this if branch with this.watcher.pause() is actually never executed (see line 252)?

webpack/lib/Watching.js

Lines 252 to 266 in f4ae05e

this.watcher = null;
if (err) {
this.compiler.modifiedFiles = undefined;
this.compiler.removedFiles = undefined;
this.compiler.fileTimestamps = undefined;
this.compiler.contextTimestamps = undefined;
return this.handler(err);
}
this.compiler.fileTimestamps = fileTimeInfoEntries;
this.compiler.contextTimestamps = contextTimeInfoEntries;
this.compiler.removedFiles = removedFiles;
this.compiler.modifiedFiles = changedFiles;
if (this.watcher) {
this.pausedWatcher = this.watcher;
this.watcher.pause();

@tu4mo
Copy link

tu4mo commented Apr 21, 2021

Any chance getting this reviewed soon?

@tu4mo
Copy link

tu4mo commented May 10, 2021

I updated to latest Gatsby and patched webpack manually with this and #12881, and in my initial tests these seem to have fixed hot reloading issues we are having in gatsbyjs/gatsby#27609.

FYI @sokra

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 skips changes to files when compilation is suspended
4 participants