Refactor plugin configuration #99
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const CleanWebpackPlugin = require('clean-webpack-plugin'); | ||
|
|
||
| /** | ||
| * @param {WebpackConfig} webpackConfig | ||
| * @param {Array} paths to clean | ||
| * @param {Object} cleanUpOptions | ||
| * @return {Array} of plugins to add to webpack | ||
| */ | ||
| module.exports = { | ||
| getPlugins(webpackConfig, paths, cleanUpOptions = {}) { | ||
|
|
||
| let config = Object.assign({}, cleanUpOptions, { | ||
| root: webpackConfig.outputPath, | ||
| verbose: false, | ||
| }); | ||
|
|
||
| return [new CleanWebpackPlugin(paths, config)]; | ||
| } | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const webpack = require('webpack'); | ||
|
|
||
| /** | ||
| * @param {WebpackConfig} webpackConfig | ||
| * @return {Array} of plugins to add to webpack | ||
| */ | ||
| module.exports = { | ||
| getPlugins(webpackConfig) { | ||
|
|
||
| return [new webpack.optimize.CommonsChunkPlugin({ | ||
| name: [ | ||
| webpackConfig.sharedCommonsEntryName, | ||
| /* | ||
| * Always dump a 2nd file - manifest.json that | ||
| * will contain the webpack manifest information. | ||
| * This changes frequently, and without this line, | ||
| * it would be packaged inside the "shared commons entry" | ||
| * file - e.g. vendor.js, which would prevent long-term caching. | ||
| */ | ||
| 'manifest' | ||
| ], | ||
| minChunks: Infinity, | ||
| })]; | ||
| } | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const DeleteUnusedEntriesJSPlugin = require('../webpack/delete-unused-entries-js-plugin'); | ||
|
|
||
| /** | ||
| * @param {WebpackConfig} webpackConfig | ||
| * @return {Array} of plugins to add to webpack | ||
| */ | ||
| module.exports = { | ||
| getPlugins(webpackConfig) { | ||
|
|
||
| return [new DeleteUnusedEntriesJSPlugin( | ||
| // transform into an Array | ||
| [... webpackConfig.styleEntries.keys()] | ||
| )]; | ||
| } | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const pathUtil = require('../config/path-util'); | ||
| const AssetOutputDisplayPlugin = require('../friendly-errors/asset-output-display-plugin'); | ||
| const FriendlyErrorsPlugin = require('./friendly-errors'); | ||
|
|
||
| /** | ||
| * @param {WebpackConfig} webpackConfig | ||
| * @param {Array} paths to clean | ||
| * @param {Object} cleanUpOptions | ||
| * @return {Array} of plugins to add to webpack | ||
| */ | ||
| module.exports = { | ||
| getPlugins(webpackConfig) { | ||
| const outputPath = pathUtil.getRelativeOutputPath(webpackConfig); | ||
| const friendlyErrorsPlugin = FriendlyErrorsPlugin.getPlugins(); | ||
| return [new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin[0])]; | ||
|
There was a problem hiding this comment. This is clever, but a problem :). Because The only idea I can think of right now, is to change // old
plugins.push(new FriendlyErrorsPlugin());
// now
plugins.push('friendly_errors_plugin', new FriendlyErrorsPlugin());then we could use that internal key to find other plugins. At the bottom of There was a problem hiding this comment. I see your point. Is this issue present in general for all plugins or it is just this particular case with the Regarding hashed solution:
I don't like it either. This case looks to me like a good scenario for a singleton. I did some research about the topic on the What do you think if we try to implement one of those solutions for that I think this is the best way we can guaranty the single instance we need. There was a problem hiding this comment.
Yes.... but no :). We allow you to generate multiple webpack configs. And to be strict about it, each separate webpack config should have its own instances of plugins. So, yes, we need to make sure that we don't create multiple There was a problem hiding this comment. To apply a solution fitting all cases known so far, I take into account:
From what we have talked I think the best solution is to not refactor this plugin at all :), we leave it as it was originally. That way we met all requirements. Still, I am sorry to go over this but I would like to know what I am missing :)
Having the previous in mind. In which way a singleton will be different to the original code when we call thanks in advance for taking the time. There was a problem hiding this comment.
Honestly, in practice, it probably makes not difference for Could we just pass in the FriendlyErrorsPlugin instance as a dependency - i.e. add a |
||
| } | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really a
dev-serverplugin... it's a "AssetOutputDisplayPlugin", which happens to be disabled when the dev-server is used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops !! my bad. I'll update that.