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 problem when updating persistent cached modules from unsafe cached modules #13837

Merged
merged 2 commits into from Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 8 additions & 6 deletions lib/NormalModule.js
Expand Up @@ -350,18 +350,20 @@ class NormalModule extends Module {
this.resource = m.resource;
this.matchResource = m.matchResource;
this.loaders = m.loaders;
this._sourceTypes = m._sourceTypes;
this._sourceSizes = m._sourceSizes;
}

/**
* Assuming this module is in the cache. Remove internal references to allow freeing some memory.
*/
cleanupForCache() {
// Make sure to cache types and sizes before cleanup
if (this._sourceTypes === undefined) this.getSourceTypes();
for (const type of this._sourceTypes) {
this.size(type);
// Make sure to cache types and sizes before cleanup when this module has been built
// They are accessed by the stats and we don't want them to crash after cleanup
// TODO reconsider this for webpack 6
if (this.buildInfo) {
if (this._sourceTypes === undefined) this.getSourceTypes();
for (const type of this._sourceTypes) {
this.size(type);
}
}
super.cleanupForCache();
this.parser = undefined;
Expand Down
41 changes: 23 additions & 18 deletions lib/asset/AssetModulesPlugin.js
Expand Up @@ -172,24 +172,29 @@ class AssetModulesPlugin {
);
if (modules) {
for (const module of modules) {
const codeGenResult = codeGenerationResults.get(
module,
chunk.runtime
);
result.push({
render: () => codeGenResult.sources.get(type),
filename:
module.buildInfo.filename ||
codeGenResult.data.get("filename"),
info:
module.buildInfo.assetInfo ||
codeGenResult.data.get("assetInfo"),
auxiliary: true,
identifier: `assetModule${chunkGraph.getModuleId(module)}`,
hash:
module.buildInfo.fullContentHash ||
codeGenResult.data.get("fullContentHash")
});
try {
const codeGenResult = codeGenerationResults.get(
module,
chunk.runtime
);
result.push({
render: () => codeGenResult.sources.get(type),
filename:
module.buildInfo.filename ||
codeGenResult.data.get("filename"),
info:
module.buildInfo.assetInfo ||
codeGenResult.data.get("assetInfo"),
auxiliary: true,
identifier: `assetModule${chunkGraph.getModuleId(module)}`,
hash:
module.buildInfo.fullContentHash ||
codeGenResult.data.get("fullContentHash")
});
} catch (e) {
e.message += `\nduring rendering of asset ${module.identifier()}`;
throw e;
}
}
}

Expand Down
22 changes: 20 additions & 2 deletions test/ConfigTestCases.template.js
Expand Up @@ -235,7 +235,7 @@ const describeCases = config => {
rimraf.sync(outputDirectory);
fs.mkdirSync(outputDirectory, { recursive: true });
const deprecationTracker = deprecationTracking.start();
require("..")(options, (err, stats) => {
const onCompiled = (err, stats) => {
const deprecations = deprecationTracker();
if (err) return handleFatalError(err, done);
const statOptions = {
Expand Down Expand Up @@ -570,7 +570,25 @@ const describeCases = config => {
done();
})
.catch(done);
});
};
if (config.cache) {
try {
const compiler = require("..")(options);
compiler.run(err => {
if (err) return handleFatalError(err, done);
compiler.run((error, stats) => {
compiler.close(err => {
if (err) return handleFatalError(err, done);
onCompiled(error, stats);
});
});
});
} catch (e) {
handleFatalError(e, done);
}
} else {
require("..")(options, onCompiled);
}
}, 30000);

const {
Expand Down
35 changes: 21 additions & 14 deletions test/configCases/asset-modules/http-url/server/index.js
Expand Up @@ -6,7 +6,10 @@ const fs = require("fs");
* @returns {Promise<import("http").Server>} server instance
*/
function createServer(port) {
const file = fs.readFileSync("./test/configCases/asset-modules/http-url/server/index.css").toString().trim();
const file = fs
.readFileSync("./test/configCases/asset-modules/http-url/server/index.css")
.toString()
.trim();

const server = http.createServer((req, res) => {
if (req.url !== "/index.css") {
Expand All @@ -18,7 +21,7 @@ function createServer(port) {
});

return new Promise((resolve, reject) => {
server.listen(port, (err) => {
server.listen(port, err => {
if (err) {
reject(err);
} else {
Expand All @@ -40,21 +43,25 @@ class ServerPlugin {
* @param {import("../../../../../").Compiler} compiler
*/
apply(compiler) {
const serverPromise = createServer(this.port);
let server;

serverPromise
.then(server => server.unref());
compiler.hooks.beforeRun.tapPromise(
"ServerPlugin",
async (compiler, callback) => {
if (!server) {
server = await createServer(this.port);
server.unref();
}
}
);

compiler.hooks.done.tapAsync("ServerPlugin", (stats, callback) => {
serverPromise
.then(server => server.close(callback))
.catch(callback)
});

compiler.hooks.beforeRun.tapAsync("ServerPlugin", (compiler, callback) => {
serverPromise
.then(() => callback())
.catch(callback)
if (server) {
server.close(callback);
server = undefined;
} else {
callback();
}
});
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/configCases/asset-modules/unsafe-cache-13827/index.js
@@ -0,0 +1,5 @@
import url from "package";

it("should create a data url", () => {
expect(url.protocol).toBe("data:");
});
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

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

@@ -0,0 +1,12 @@
/** @type {import("../../../../").Configuration} */
module.exports = {
mode: "development",
module: {
rules: [
{
dependency: "url",
type: "asset"
}
]
}
};
15 changes: 11 additions & 4 deletions test/configCases/clean/enabled/webpack.config.js
Expand Up @@ -10,12 +10,19 @@ module.exports = {
},
plugins: [
compiler => {
let once = true;
compiler.hooks.thisCompilation.tap("Test", compilation => {
compilation.hooks.processAssets.tap("Test", assets => {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(outputPath, "this/dir/should/be/removed");
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
if (once) {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(
outputPath,
"this/dir/should/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
once = false;
}
assets["this/dir/should/not/be/removed/file.ext"] = new RawSource("");
});
});
Expand Down
27 changes: 17 additions & 10 deletions test/configCases/clean/ignore-fn/webpack.config.js
Expand Up @@ -14,18 +14,25 @@ module.exports = {
},
plugins: [
compiler => {
let once = true;
compiler.hooks.thisCompilation.tap("Test", compilation => {
compilation.hooks.processAssets.tap("Test", assets => {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(outputPath, "this/dir/should/be/removed");
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
if (once) {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(
outputPath,
"this/dir/should/be/removed"
);
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
once = false;
}
assets["this/dir/should/not/be/removed/file.ext"] = new RawSource("");
});
});
Expand Down
39 changes: 23 additions & 16 deletions test/configCases/clean/ignore-hook/webpack.config.js
Expand Up @@ -11,6 +11,7 @@ module.exports = {
},
plugins: [
compiler => {
let once = true;
compiler.hooks.thisCompilation.tap("Test", compilation => {
webpack.CleanPlugin.getCompilationHooks(compilation).keep.tap(
"Test",
Expand All @@ -20,22 +21,28 @@ module.exports = {
}
);
compilation.hooks.processAssets.tap("Test", assets => {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(outputPath, "this/dir/should/be/removed");
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
const ignoredTooDir = path.join(
outputPath,
"this/is/ignored/too/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
fs.mkdirSync(ignoredTooDir, { recursive: true });
fs.writeFileSync(path.join(ignoredTooDir, "file.ext"), "");
if (once) {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(
outputPath,
"this/dir/should/be/removed"
);
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
const ignoredTooDir = path.join(
outputPath,
"this/is/ignored/too/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
fs.mkdirSync(ignoredTooDir, { recursive: true });
fs.writeFileSync(path.join(ignoredTooDir, "file.ext"), "");
once = false;
}
assets["this/dir/should/not/be/removed/file.ext"] = new RawSource("");
});
});
Expand Down
27 changes: 17 additions & 10 deletions test/configCases/clean/ignore-rx/webpack.config.js
Expand Up @@ -12,18 +12,25 @@ module.exports = {
},
plugins: [
compiler => {
let once = true;
compiler.hooks.thisCompilation.tap("Test", compilation => {
compilation.hooks.processAssets.tap("Test", assets => {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(outputPath, "this/dir/should/be/removed");
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
if (once) {
const outputPath = compilation.getPath(compiler.outputPath, {});
const customDir = path.join(
outputPath,
"this/dir/should/be/removed"
);
const ignoredDir = path.join(
outputPath,
"this/is/ignored/dir/that/should/not/be/removed"
);
fs.mkdirSync(customDir, { recursive: true });
fs.writeFileSync(path.join(customDir, "file.ext"), "");
fs.mkdirSync(ignoredDir, { recursive: true });
fs.writeFileSync(path.join(ignoredDir, "file.ext"), "");
once = false;
}
assets["this/dir/should/not/be/removed/file.ext"] = new RawSource("");
});
});
Expand Down