Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

feat: custom minify function #321

Closed
wants to merge 1 commit into from

Conversation

alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Jun 28, 2018

Allow to use uglify-js and terser.

Next steps:

  1. Update webpack-defaults.
  2. Refactor.
  3. Switch on terser., update tests and docs.
  4. Major release (drop support nodejs@4).

README.md Outdated
@@ -46,6 +46,7 @@ module.exports = {
|**`cache`**|`{Boolean\|String}`|`false`|Enable file caching|
|**`parallel`**|`{Boolean\|Number}`|`false`|Use multi-process parallel running to improve the build speed|
|**`sourceMap`**|`{Boolean}`|`false`|Use source maps to map error message locations to modules (This slows down the compilation) ⚠️ **`cheap-source-map` options don't work with this plugin**|
|**`minify`**|`{Function}`|`(file, uglifyOptions) => uglify.minify(file, uglifyOptions)`|Allows you to override default minify function|
Copy link
Contributor

Choose a reason for hiding this comment

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

The second param should be named differently

Choose a reason for hiding this comment

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

seconded. (this repo could really benefit from a webpack-defaults update)

Copy link
Member Author

Choose a reason for hiding this comment

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

uglifyOptions -> options?

Copy link
Contributor

Choose a reason for hiding this comment

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

options seems good

Copy link
Member Author

Choose a reason for hiding this comment

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

@ooflorent maybe better minifyOptions, because it will be misleading between options for plugin and options for minification

Choose a reason for hiding this comment

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

yeah I could see that being a point of possible confusion

@alexander-akait
Copy link
Member Author

Close in favor #325

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants