Feature configure manifest options #142
Conversation
index.js
Outdated
| * }) | ||
| * | ||
| * @param {Object} options | ||
| * @returns {publicApi} |
lib/plugins/manifest.js
Outdated
| basePath: manifestPrefix, | ||
| // guarantee the value uses the public path (or CDN public path) | ||
| publicPath: webpackConfig.getRealPublicPath(), | ||
| // always write a manifest.json file, even with webpack-dev-server | ||
| writeToFileEmit: true, | ||
| })); | ||
| }; | ||
| let manifestConfig = Object.assign(defaultConfig, webpackConfig.manifestPluginOptions); |
There was a problem hiding this comment.
Just a minor remark here, Object.assign modifies its first argument, so in this case defaultConfig (which doesn't really makes sense since it won't be the default config anymore after that).
You probably could either do:
const defaultConfig = { /* ... */ };
const manifestConfig = Object.assign({}, defaultConfig, webpackConfig.manifestPluginOptions);Or only use a single variable (which looks a bit less clear in my opinion):
const manifestConfig = Object.assign({}, { /* ... */ }, webpackConfig.manifestPluginOptions);
There was a problem hiding this comment.
More globally: I'm not sure that the configureManifestPlugin() method should take an object as an argument.
For other methods related to loaders/plugins (e.g. enableSassLoader(), configureBabel(), enableForkedTypeScriptTypesChecking(), ...) a callback is used to modify default options.
|
@Lyrkan I tried to look into the other features pull request but honestly I am kind of lost with how this works. Could you tell me if this is more suitable ? |
lib/WebpackConfig.js
Outdated
| @@ -117,6 +118,16 @@ class WebpackConfig { | |||
| this.manifestKeyPrefix = manifestKeyPrefix; | |||
| } | |||
|
|
|||
| configureManifestPlugin(manifestPluginOptionsCallback = () => {}) { | |||
| this.usePostCssLoader = true; | |||
There was a problem hiding this comment.
You can probably remove that line :)
lib/WebpackConfig.js
Outdated
| this.usePostCssLoader = true; | ||
|
|
||
| if (typeof manifestPluginOptionsCallback !== 'function') { | ||
| throw new Error('Argument 1 to enablePostCssLoader() must be a callback function.'); |
There was a problem hiding this comment.
Argument 1 to configureManifestPlugin() must be a callback function.
Maybe you could also add a test for that part (in test/WebpackConfig.js).
|
Should be good now, thank you for your help |
|
This is excellent work! Thank you @Seikyo! |
This PR adds
configureManifestPlugin()as discussed in #125Example:
These options will override any default option.