Skip to content

Commit

Permalink
fallback to normal snapshotting when managed path optimization fails
Browse files Browse the repository at this point in the history
  • Loading branch information
sokra committed Nov 15, 2021
1 parent 98ea582 commit cae22d1
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 33 deletions.
56 changes: 41 additions & 15 deletions lib/FileSystemInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -1993,7 +1993,6 @@ class FileSystemInfo {
if (managedItem) {
managedItems.add(managedItem);
managedSet.add(path);
managedFiles.add(join(this.fs, managedItem, "package.json"));
return true;
}
}
Expand All @@ -2004,7 +2003,6 @@ class FileSystemInfo {
if (managedItem) {
managedItems.add(managedItem);
managedSet.add(path);
managedFiles.add(join(this.fs, managedItem, "package.json"));
return true;
}
}
Expand All @@ -2018,8 +2016,7 @@ class FileSystemInfo {
}
return capturedItems;
};
if (files) {
const capturedFiles = captureNonManaged(files, managedFiles);
const processCapturedFiles = capturedFiles => {
switch (mode) {
case 3:
this._fileTshsOptimization.optimize(snapshot, capturedFiles);
Expand Down Expand Up @@ -2096,12 +2093,11 @@ class FileSystemInfo {
}
break;
}
};
if (files) {
processCapturedFiles(captureNonManaged(files, managedFiles));
}
if (directories) {
const capturedDirectories = captureNonManaged(
directories,
managedContexts
);
const processCapturedDirectories = capturedDirectories => {
switch (mode) {
case 3:
this._contextTshsOptimization.optimize(snapshot, capturedDirectories);
Expand Down Expand Up @@ -2221,9 +2217,13 @@ class FileSystemInfo {
}
break;
}
};
if (directories) {
processCapturedDirectories(
captureNonManaged(directories, managedContexts)
);
}
if (missing) {
const capturedMissing = captureNonManaged(missing, managedMissing);
const processCapturedMissing = capturedMissing => {
this._missingExistenceOptimization.optimize(snapshot, capturedMissing);
for (const path of capturedMissing) {
const cache = this._fileTimestamps.get(path);
Expand All @@ -2248,11 +2248,15 @@ class FileSystemInfo {
});
}
}
};
if (missing) {
processCapturedMissing(captureNonManaged(missing, managedMissing));
}
this._managedItemInfoOptimization.optimize(snapshot, managedItems);
for (const path of managedItems) {
const cache = this._managedItems.get(path);
if (cache !== undefined) {
managedFiles.add(join(this.fs, path, "package.json"));
managedItemInfo.set(path, cache);
} else {
jobs++;
Expand All @@ -2264,9 +2268,24 @@ class FileSystemInfo {
);
}
jobError();
} else {
} else if (entry) {
managedFiles.add(join(this.fs, path, "package.json"));
managedItemInfo.set(path, entry);
jobDone();
} else {
// Fallback to normal snapshotting
const process = (set, fn) => {
if (set.size === 0) return;
const captured = new Set();
for (const file of set) {
if (file.startsWith(path)) captured.add(file);
}
if (captured.size > 0) fn(captured);
};
process(managedFiles, processCapturedFiles);
process(managedContexts, processCapturedDirectories);
process(managedMissing, processCapturedMissing);
jobDone();
}
});
}
Expand Down Expand Up @@ -3479,9 +3498,10 @@ class FileSystemInfo {
this._managedItems.set(path, "nested");
return callback(null, "nested");
}
const problem = `Managed item ${path} isn't a directory or doesn't contain a package.json`;
this.logger.warn(problem);
return callback(new Error(problem));
this.logger.warn(
`Managed item ${path} isn't a directory or doesn't contain a package.json (see snapshot.managedPaths option)`
);
return callback();
});
return;
}
Expand All @@ -3493,6 +3513,12 @@ class FileSystemInfo {
} catch (e) {
return callback(e);
}
if (!data.name) {
this.logger.warn(
`${packageJsonPath} doesn't contain a "name" property (see snapshot.managedPaths option)`
);
return callback();
}
const info = `${data.name || ""}@${data.version || ""}`;
this._managedItems.set(path, info);
callback(null, info);
Expand Down
79 changes: 70 additions & 9 deletions test/BuildDependencies.longtest.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,49 @@ const exec = (n, options = {}) => {
p.stderr.on("data", chunk => chunks.push(chunk));
p.stdout.on("data", chunk => chunks.push(chunk));
p.once("exit", code => {
const stdout = chunks.join("");
const errors = [];
const warnings = [];
const rawStdout = chunks.join("");
const stdout = rawStdout.replace(
// This warning is expected
/<([ew])> \[.+\n(?:<([ew])> [^[].+\n)*/g,
(message, type) => {
(type === "e" ? errors : warnings).push(message);
return "";
}
);
if (errors.length > 0) {
return reject(
new Error(
`Unexpected errors in ${n} output:\n${errors.join(
"\n"
)}\n\n${rawStdout}`
)
);
}
for (const regexp of options.warnings || []) {
const idx = warnings.findIndex(w => regexp.test(w));
if (idx < 0) {
return reject(
new Error(
`Warning ${regexp} was not found in ${n} output:\n${rawStdout}`
)
);
}
warnings.splice(idx, 1);
}
if (warnings.length > 0) {
return reject(
new Error(
`Unexpected warnings in ${n} output:\n${warnings.join(
"\n"
)}\n\n${rawStdout}`
)
);
}
if (code === 0) {
if (!options.ignoreErrors && /<[ew]>/.test(stdout))
return reject(stdout);
return reject(new Error(stdout));
resolve(stdout);
} else {
reject(new Error(`Code ${code}: ${stdout}`));
Expand Down Expand Up @@ -99,7 +138,7 @@ describe("BuildDependencies", () => {
await exec("0", {
invalidBuildDepdencies: true,
buildTwice: true,
ignoreErrors: true
warnings: [/Can't resolve 'should-fail-resolving'/]
});
fs.writeFileSync(
path.resolve(inputDirectory, "loader-dependency.js"),
Expand All @@ -113,13 +152,21 @@ describe("BuildDependencies", () => {
path.resolve(inputDirectory, "esm-dependency.js"),
"module.exports = 1;"
);
await exec("1");
await exec("1", {
warnings: supportsEsm && [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
fs.writeFileSync(
path.resolve(inputDirectory, "loader-dependency.js"),
"module.exports = Date.now();"
);
const now1 = Date.now();
const output2 = await exec("2");
const output2 = await exec("2", {
warnings: supportsEsm && [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
expect(output2).toMatch(/but build dependencies have changed/);
expect(output2).toMatch(/Captured build dependencies/);
expect(output2).not.toMatch(/Assuming/);
Expand All @@ -137,7 +184,11 @@ describe("BuildDependencies", () => {
version: "2.0.0"
})
);
const output4 = await exec("4");
const output4 = await exec("4", {
warnings: supportsEsm && [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
expect(output4).toMatch(/resolving of build dependencies is invalid/);
expect(output4).not.toMatch(/but build dependencies have changed/);
expect(output4).toMatch(/Captured build dependencies/);
Expand All @@ -146,7 +197,11 @@ describe("BuildDependencies", () => {
"module.exports = Date.now();"
);
const now2 = Date.now();
await exec("5");
await exec("5", {
warnings: supportsEsm && [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
const now3 = Date.now();
await exec("6");
await exec("7", {
Expand All @@ -160,7 +215,10 @@ describe("BuildDependencies", () => {
);
now4 = Date.now();
await exec("8", {
definedValue: "other"
definedValue: "other",
warnings: [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
fs.writeFileSync(
path.resolve(inputDirectory, "esm-async-dependency.mjs"),
Expand All @@ -169,7 +227,10 @@ describe("BuildDependencies", () => {
now5 = Date.now();

await exec("9", {
definedValue: "other"
definedValue: "other",
warnings: [
/Managed item .+dep-without-package\.json isn't a directory or doesn't contain a package\.json/
]
});
}
const results = Array.from({ length: supportsEsm ? 10 : 8 }).map((_, i) =>
Expand Down
4 changes: 0 additions & 4 deletions test/ConfigCacheTestCases.longtest.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const path = require("path");
const { describeCases } = require("./ConfigTestCases.template");

describeCases({
Expand All @@ -8,8 +7,5 @@ describeCases({
buildDependencies: {
defaultWebpack: []
}
},
snapshot: {
managedPaths: [path.resolve(__dirname, "../node_modules")]
}
});
9 changes: 5 additions & 4 deletions test/ConfigTestCases.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,11 @@ const describeCases = config => {
...config.cache
};
}
if (config.snapshot) {
options.snapshot = {
...config.snapshot
};
if (!options.snapshot) options.snapshot = {};
if (!options.snapshot.managedPaths) {
options.snapshot.managedPaths = [
path.resolve(__dirname, "../node_modules")
];
}
});
testConfig = {
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import "./loader!package";

it("should compile and run the test in config", () => {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const path = require("path");

/** @type {import("../../../../").LoaderDefinition} */
module.exports = function (source) {
this.addDependency(path.resolve(__dirname, "node_modules/package/extra.js"));
this.addDependency(path.resolve(__dirname, "extra.js"));
return source;
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const path = require("path");

/** @type {import("../../../../").Configuration} */
module.exports = {
snapshot: {
managedPaths: [path.resolve(__dirname, "node_modules")]
},
plugins: [
compiler => {
compiler.hooks.done.tap("Test", ({ compilation }) => {
const fileDeps = Array.from(compilation.fileDependencies);
expect(fileDeps).toContain(
path.resolve(__dirname, "node_modules/package/index.js")
);
expect(fileDeps).toContain(
path.resolve(__dirname, "node_modules/package/extra.js")
);
expect(fileDeps).toContain(
path.resolve(__dirname, "node_modules/package/package.json")
);
expect(fileDeps).toContain(path.resolve(__dirname, "extra.js"));
expect(fileDeps).toContain(path.resolve(__dirname, "index.js"));
});
}
],
module: {
unsafeCache: true
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,8 @@ module.exports = {
expect(fileDeps).toContain(path.resolve(__dirname, "index.js"));
});
}
]
],
module: {
unsafeCache: false
}
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions test/fixtures/buildDependencies/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ function run({ default: value2, asyncDep: value3 }) {
level: "verbose",
debug: /PackFile/
},
snapshot: {
// TODO remove webpack 6
managedPaths: [/^(.+?[\\/]node_modules[\\/])/]
},
cache: {
type: "filesystem",
cacheDirectory: path.resolve(__dirname, "../../js/buildDepsCache"),
Expand Down

0 comments on commit cae22d1

Please sign in to comment.