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

feat: watch mode config change #2797

Closed
wants to merge 5 commits into from
Closed

feat: watch mode config change #2797

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6d3ba0d
feat: watch mode config change
evenstensberg Jun 24, 2021
05703b0
chore: remove unneded deps
evenstensberg Jun 25, 2021
bc1f853
chore: remove console.logs
evenstensberg Jun 25, 2021
ae86f43
chore: check for existence of cliplugin
evenstensberg Jun 25, 2021
bf73f5b
chore: use new version of watch traverse
evenstensberg Jul 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions packages/webpack-cli/lib/webpack-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ class WebpackCLI {
),
});
}

async tryRequireThenImport(module, handleError = true) {
let result;

try {
delete require.cache[module];
Copy link
Member

Choose a reason for hiding this comment

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

Unsafe and very bad for perf

Copy link
Member Author

Choose a reason for hiding this comment

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

its cache for one file, perf wont change that much because of this

result = require(module);
console.log(result);
} catch (error) {
console.log(error, "HEY");
let previousModuleCompile;

// TODO Workaround https://github.com/zertosh/v8-compile-cache/issues/30
Expand Down Expand Up @@ -67,7 +69,6 @@ class WebpackCLI {
throw error;
}
}

// For babel/typescript
if (result.default) {
result = result.default;
Expand Down Expand Up @@ -1545,10 +1546,8 @@ class WebpackCLI {

process.exit(2);
}

return { options, path: configPath };
};

const evaluateConfig = async (loadedConfig, argv) => {
const isMultiCompiler = Array.isArray(loadedConfig.options);
const config = isMultiCompiler ? loadedConfig.options : [loadedConfig.options];
Expand Down Expand Up @@ -1581,7 +1580,7 @@ class WebpackCLI {
return loadedConfig;
};

const config = { options: {}, path: new WeakMap() };
const config = { options: {}, path: new Map() };
Copy link
Member

Choose a reason for hiding this comment

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

Leaking

Copy link
Member Author

Choose a reason for hiding this comment

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

We're not using config.path anywhere, what's to leak?

Copy link
Member

Choose a reason for hiding this comment

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

Look at CLI plugin


if (options.config && options.config.length > 0) {
const evaluatedConfigs = await Promise.all(
Expand Down Expand Up @@ -1638,9 +1637,7 @@ class WebpackCLI {
if (foundDefaultConfigFile) {
const loadedConfig = await loadConfig(foundDefaultConfigFile.path);
const evaluatedConfig = await evaluateConfig(loadedConfig, options.argv || {});

config.options = evaluatedConfig.options;

if (Array.isArray(config.options)) {
config.options.forEach((options) => {
config.path.set(options, evaluatedConfig.path);
Expand Down Expand Up @@ -2041,9 +2038,7 @@ class WebpackCLI {

async createCompiler(options, callback) {
this.applyNodeEnv(options);

let config = await this.resolveConfig(options);

config = await this.applyOptions(config, options);
config = await this.applyCLIPlugin(config, options);

Expand Down Expand Up @@ -2077,7 +2072,7 @@ class WebpackCLI {
if (compiler && compiler.compiler) {
compiler = compiler.compiler;
}

compiler.config = config;
return compiler;
}

Expand Down Expand Up @@ -2171,7 +2166,15 @@ class WebpackCLI {
}

compiler = await this.createCompiler(options, callback);

if (options.watch) {
// eslint-disable-next-line
for (const [_, filePath] of compiler.config.path.entries()) {
Copy link
Member

Choose a reason for hiding this comment

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

You can do the same using WeakMap

Copy link
Member Author

Choose a reason for hiding this comment

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

cant iterate over a weakmap

Copy link
Member

Choose a reason for hiding this comment

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

You have configuration keys

// eslint-disable-next-line
fs.watchFile(filePath, async () => {
compiler = await this.createCompiler(options, callback);
Copy link
Member

Choose a reason for hiding this comment

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

It should rerun commander, not create compiler

});
}
}
if (!compiler) {
return;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/webpack-cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@
"@webpack-cli/configtest": "^1.0.4",
"@webpack-cli/info": "^1.3.0",
"@webpack-cli/serve": "^1.5.1",
"async-require": "^1.2.2",
evenstensberg marked this conversation as resolved.
Show resolved Hide resolved
"colorette": "^1.2.1",
"commander": "^7.0.0",
"execa": "^5.0.0",
"fastest-levenshtein": "^1.0.12",
"import-local": "^3.0.2",
"interpret": "^2.2.0",
"rechoir": "^0.7.0",
"require.async": "^0.1.1",
evenstensberg marked this conversation as resolved.
Show resolved Hide resolved
"v8-compile-cache": "^2.2.0",
"webpack-merge": "^5.7.3"
},
Expand Down