Skip to content

Commit

Permalink
Merge pull request #13399 from webpack/bugfix/resume-watching
Browse files Browse the repository at this point in the history 
fix suspend and resuming watching correctly
  • Loading branch information
@sokra
sokra committed May 19, 2021
2 parents ac2973c + 23f492f commit 0b3f717
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 38 deletions.
103 changes: 71 additions & 32 deletions lib/Watching.js
View file
Expand Up @@ -57,16 +57,53 @@ class Watching {
this._needRecords = true;
this.watcher = undefined;
this.pausedWatcher = undefined;
/** @type {Set<string>} */
this._collectedChangedFiles = undefined;
/** @type {Set<string>} */
this._collectedRemovedFiles = undefined;
this._done = this._done.bind(this);
process.nextTick(() => {
if (this._initial) this._invalidate();
});
}

_go() {
_mergeWithCollected(changedFiles, removedFiles) {
if (!changedFiles) return;
if (!this._collectedChangedFiles) {
this._collectedChangedFiles = new Set(changedFiles);
this._collectedRemovedFiles = new Set(removedFiles);
} else {
for (const file of changedFiles) {
this._collectedChangedFiles.add(file);
this._collectedRemovedFiles.delete(file);
}
for (const file of removedFiles) {
this._collectedChangedFiles.delete(file);
this._collectedRemovedFiles.add(file);
}
}
}

_go(fileTimeInfoEntries, contextTimeInfoEntries, changedFiles, removedFiles) {
this._initial = false;
this.startTime = Date.now();
this.running = true;
if (this.watcher) {
this.pausedWatcher = this.watcher;
this.watcher.pause();
this.watcher = null;
}
this._mergeWithCollected(
changedFiles ||
(this.pausedWatcher &&
this.pausedWatcher.getAggregatedChanges &&
this.pausedWatcher.getAggregatedChanges()),
(this.compiler.removedFiles =
removedFiles ||
(this.pausedWatcher &&
this.pausedWatcher.getAggregatedRemovals &&
this.pausedWatcher.getAggregatedRemovals()))
);
const run = () => {
if (this.compiler.idle) {
return this.compiler.cache.endIdle(err => {
Expand All @@ -83,6 +120,19 @@ class Watching {
run();
});
}

this.compiler.modifiedFiles = this._collectedChangedFiles;
this._collectedChangedFiles = undefined;
this.compiler.removedFiles = this._collectedRemovedFiles;
this._collectedRemovedFiles = undefined;

this.compiler.fileTimestamps =
fileTimeInfoEntries ||
(this.pausedWatcher && this.pausedWatcher.getFileTimeInfoEntries());
this.compiler.contextTimestamps =
contextTimeInfoEntries ||
(this.pausedWatcher && this.pausedWatcher.getContextTimeInfoEntries());

this.invalid = false;
this._invalidReported = false;
this.compiler.hooks.watchRun.callAsync(this.compiler, err => {
Expand Down Expand Up @@ -254,25 +304,19 @@ class Watching {
changedFiles,
removedFiles
) => {
this.pausedWatcher = this.watcher;
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();
this._invalidate(
fileTimeInfoEntries,
contextTimeInfoEntries,
changedFiles,
removedFiles
);
this._onChange();
},
(fileName, changeTime) => {
Expand Down Expand Up @@ -301,31 +345,26 @@ class Watching {
this._invalidate();
}

_invalidate() {
if (this.suspended) return;
if (this._isBlocked()) {
this.blocked = true;
_invalidate(
fileTimeInfoEntries,
contextTimeInfoEntries,
changedFiles,
removedFiles
) {
if (this.suspended || (this._isBlocked() && (this.blocked = true))) {
this._mergeWithCollected(changedFiles, removedFiles);
return;
}
if (this.watcher) {
this.compiler.modifiedFiles =
this.watcher.getAggregatedChanges &&
this.watcher.getAggregatedChanges();
this.compiler.removedFiles =
this.watcher.getAggregatedRemovals &&
this.watcher.getAggregatedRemovals();
this.compiler.fileTimestamps = this.watcher.getFileTimeInfoEntries();
this.compiler.contextTimestamps =
this.watcher.getContextTimeInfoEntries();
this.pausedWatcher = this.watcher;
this.watcher.pause();
this.watcher = null;
}

if (this.running) {
this.invalid = true;
} else {
this._go();
this._go(
fileTimeInfoEntries,
contextTimeInfoEntries,
changedFiles,
removedFiles
);
}
}

Expand Down
23 changes: 19 additions & 4 deletions lib/node/NodeWatchFileSystem.js
View file
Expand Up @@ -70,11 +70,12 @@ class NodeWatchFileSystem {
}
this.watcher.once("aggregated", (changes, removals) => {
if (this.inputFileSystem && this.inputFileSystem.purge) {
const fs = this.inputFileSystem;
for (const item of changes) {
this.inputFileSystem.purge(item);
fs.purge(item);
}
for (const item of removals) {
this.inputFileSystem.purge(item);
fs.purge(item);
}
}
const times = this.watcher.getTimeInfoEntries();
Expand All @@ -99,10 +100,24 @@ class NodeWatchFileSystem {
}
},
getAggregatedRemovals: () => {
return this.watcher && this.watcher.aggregatedRemovals;
const items = this.watcher && this.watcher.aggregatedRemovals;
if (items && this.inputFileSystem && this.inputFileSystem.purge) {
const fs = this.inputFileSystem;
for (const item of items) {
fs.purge(item);
}
}
return items;
},
getAggregatedChanges: () => {
return this.watcher && this.watcher.aggregatedChanges;
const items = this.watcher && this.watcher.aggregatedChanges;
if (items && this.inputFileSystem && this.inputFileSystem.purge) {
const fs = this.inputFileSystem;
for (const item of items) {
fs.purge(item);
}
}
return items;
},
getFileTimeInfoEntries: () => {
if (this.watcher) {
Expand Down
81 changes: 81 additions & 0 deletions test/WatchSuspend.test.js
View file
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 All @@ -45,6 +49,7 @@ describe("WatchSuspend", () => {
}
});
watching = compiler.watch({ aggregateTimeout: 50 }, () => {});

compiler.hooks.done.tap("WatchSuspendTest", () => {
if (onChange) onChange();
});
Expand Down Expand Up @@ -92,5 +97,81 @@ describe("WatchSuspend", () => {
};
watching.resume();
});

for (const changeBefore of [false, true])
for (const delay of [200, 1500]) {
// eslint-disable-next-line no-loop-func
it(`should not ignore changes during resumed compilation (changeBefore: ${changeBefore}, delay: ${delay}ms)`, async () => {
// aggregateTimeout must be long enough for this test
// So set-up new watcher and wait when initial compilation is done
await new Promise(resolve => {
watching.close(() => {
watching = compiler.watch({ aggregateTimeout: 1000 }, () => {
resolve();
});
});
});
return new Promise(resolve => {
if (changeBefore) fs.writeFileSync(filePath, "'bar'", "utf-8");
setTimeout(() => {
watching.suspend();
fs.writeFileSync(filePath, "'baz'", "utf-8");

onChange = "throw";
setTimeout(() => {
onChange = () => {
expect(fs.readFileSync(outputPath, "utf-8")).toContain(
"'baz'"
);
expect(
compiler.modifiedFiles &&
Array.from(compiler.modifiedFiles).sort()
).toEqual([filePath]);
expect(
compiler.removedFiles && Array.from(compiler.removedFiles)
).toEqual([]);
onChange = null;
resolve();
};
watching.resume();
}, delay);
}, 200);
});
});
}

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);
};
});
});
});
4 changes: 2 additions & 2 deletions types.d.ts
View file
Expand Up @@ -11596,8 +11596,8 @@ declare abstract class Watching {
};
compiler: Compiler;
running: boolean;
watcher: any;
pausedWatcher: any;
watcher?: null | Watcher;
pausedWatcher?: null | Watcher;
watch(
files: Iterable<string>,
dirs: Iterable<string>,
Expand Down

0 comments on commit 0b3f717

Please sign in to comment.