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.

Sign up for GitHub

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

Jump to bottom

fix: do not skip changes when compilation is suspended #12897

Closed
wants to merge 2 commits into from
Closed

fix: do not skip changes when compilation is suspended #12897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b8d5d6
add failing test
vladar Mar 15, 2021
cc733ab
fix: do not skip changes when compiling is suspended
vladar Mar 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/Watching.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

this.watcher = null;
}
this._invalidate();
if (!this.suspended) {
this._invalidate();
} else {
this.watch(files, dirs, missing);
}
this._onChange();
},
(fileName, changeTime) => {
Expand Down
38 changes: 38 additions & 0 deletions test/WatchSuspend.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ describe("WatchSuspend", () => {
"temp-watch-" + Date.now()
);
const filePath = path.join(fixturePath, "file.js");
const file2Path = path.join(fixturePath, "file2.js");
const file3Path = path.join(fixturePath, "file3.js");
const outputPath = path.join(fixturePath, "bundle.js");
let compiler = null;
let watching = null;
Expand All @@ -33,6 +35,8 @@ describe("WatchSuspend", () => {
}
try {
fs.writeFileSync(filePath, "'foo'", "utf-8");
fs.writeFileSync(file2Path, "'file2'", "utf-8");
fs.writeFileSync(file3Path, "'file3'", "utf-8");
} catch (e) {
// skip
}
Expand Down Expand Up @@ -92,5 +96,39 @@ describe("WatchSuspend", () => {
};
watching.resume();
});

it("should not drop changes when suspended", done => {
const aggregateTimeout = 50;
// Trigger initial compilation with file2.js (assuming correct)
fs.writeFileSync(
filePath,
'require("./file2.js"); require("./file3.js")',
"utf-8"
);

onChange = () => {
// Initial compilation is done, start the test
watching.suspend();

// Trigger the first change (works as expected):
fs.writeFileSync(file2Path, "'foo'", "utf-8");

// Trigger the second change _after_ aggregation timeout of the first
setTimeout(() => {
fs.writeFileSync(file3Path, "'bar'", "utf-8");

// Wait when the file3 edit is settled and re-compile
setTimeout(() => {
watching.resume();

onChange = () => {
onChange = null;
expect(fs.readFileSync(outputPath, "utf-8")).toContain("'bar'");
done();
};
}, 200);
}, aggregateTimeout + 50);
};
});
});
});