refactor: rename `babili` to `babel-minify` #55

boopathi · 2017-08-16T10:13:36Z

No description provided.

codecov · 2017-08-16T10:21:13Z

Codecov Report

Merging #55 into master will decrease coverage by 6.91%.
The diff coverage is 63.88%.

@@           Coverage Diff           @@
##           master   #55      +/-   ##
=======================================
- Coverage   72.91%   66%   -6.92%     
=======================================
  Files           2     2              
  Lines          48    50       +2     
  Branches       16    17       +1     
=======================================
- Hits           35    33       -2     
- Misses         12    15       +3     
- Partials        1     2       +1

Impacted Files	Coverage Δ
src/index.js	`67.34% <63.88%> (-7.13%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0a724c...d788353. Read the comment docs.

pi0 · 2017-08-16T11:00:58Z

Hi! Is there any interest on this?

michael-ciniawsky · 2017-08-16T10:59:46Z

README.md

@@ -50,17 +50,16 @@ module.exports = {
 + `sourceMap`: Default: uses [webpackConfig.devtool](https://webpack.js.org/configuration/devtool/). Set this to override that.
 + `parserOpts`: Configure babel with special parser options.
 + `babel`: Pass in a custom babel-core instead. `require("babel-core")`
-+ `babili`: Pass in a custom babili preset instead - `require("babel-preset-babili")`.
-``
+ `minifyPreset`: Pass in a custom minify preset instead - `require("babel-preset-minify")`.


What about minify|minifier || preset instead?

minify or minifier sends wrong information. For example, when require("babel-minify") is passed. IMO, minifyPreset says that it's babel-preset-minify and is clear.

michael-ciniawsky · 2017-08-16T11:01:32Z

README.md

 module.exports = {
  entry: //...,
  output: //...,
  plugins: [
-    new BabiliPlugin(babiliOptions, overrides)
+    new MinifyPlugin(minifyOptions, overrides)


new MinifyPlugin(minifyOptions, pluginOptions)

overrides=> pluginOptions ?

michael-ciniawsky · 2017-08-16T11:10:57Z

src/index.js

-    this.babiliOpts = babiliOpts;
+export default class BabelMinifyPlugin {
+  constructor(minifyOpts = {}, options = {}) {
+    this.minifyOpts = minifyOpts;


constructor (options) { this.options = { parser: ...parserOptions preset: require('babel-preset-babili') minify: ...minifierOptions babel: require('babel-core'), comments: '', sourceMap: '' } }

?

Maybe instead of preset && minify just

=> preset: [ require('babel-preset-minify'), { ...minifyOptions } ] {Array}
=> preset: { ...minifyOptions } {Object}

pi0 · 2017-08-16T11:16:59Z

Thanks @michael-ciniawsky for fast review :)

michael-ciniawsky · 2017-08-16T11:24:30Z

I added the semver:major label to indicate it as a breaking change, but not really important since it will be publish under a new name on npm anyways :)

@boopathi How important is it being able to customize the babel & babel-preset-minify version via options ? :) What about a peerDependency instead?

I started porting parallelization support for this plugin from uglifyjs-webpack-plugin and finish it after has PR has landed :D

alexander-akait · 2017-08-16T11:26:59Z

@michael-ciniawsky let's fix all issues related to cache and parallelization in uglifyjs-webpack-plugin (and add tests) and we can implement same algorithm here 👍

boopathi · 2017-08-16T11:59:07Z

@boopathi How important is it being able to customize the babel & babel-preset-minify version via options ? :) What about a peerDependency instead?

Babel7 is in alpha now and almost everyone uses babel 6 in their toolchain. So it is important to have the ability to configure babel - i.e use babel7 for minification and babel6 for other transpilation or vice-versa.
babel and babel-minify are dependencies (and not peer deps) to have a good default and is the common case.

I added the semver:major label to indicate it as a breaking change, but not really important since it will be publish under a new name on npm anyways :)

I'll be publishing it as 0.2.0, to use the same version numbers as babel-minify.

michael-ciniawsky · 2017-08-16T15:33:27Z

src/index.js

-
-  apply(compiler) {
-    const { babiliOpts, options } = this;
+// alias for undefined / void 0 :(


:D Please use // eslint-disable-next-line (L12)

michael-ciniawsky · 2017-08-16T15:35:07Z

src/index.js

+      // compiler.options.devtool overrides options.sourceMap if NOT set
+      // so we set it to undefined/void 0 as the default value
+      sourceMap: getDefault(pluginOpts.sourceMap, undef),
+      jsregex: pluginOpts.jsregex || /\.js($|\?)/i,


test is better imho, since webpack uses this name all over the place for filtering files :)

michael-ciniawsky · 2017-08-16T15:43:43Z

src/index.js


-    const jsregex = options.test || /\.js($|\?)/i;
-    const commentsRegex = typeof options.comments === 'undefined' ? /@preserve|@licen(s|c)e/ : options.comments;
+function getDefault(actualValue, defaultValue) {


// eslint-disable-next-line const getDefault = (actual, default) => actual !== undefined ? actual : default

michael-ciniawsky · 2017-08-16T15:48:55Z

src/index.js

+
+            // not a ternary because prettier puts test and consequent on different lines
+            // and webpack-defaults has a rule not to do the same :(
+            if (result.map) {


// eslint-disable ${rule-name} :) compilation.assets[file] = result.map ? new SourceMapSource(result.code, file, result.map, input, inputSourceMap); : new RawSource(result.code);

michael-ciniawsky · 2017-08-16T15:52:24Z

src/index.js

+    this.options = {
+      parserOpts: pluginOpts.parserOpts || {},
+      minifyPreset: pluginOpts.minifyPreset || babelPresetMinify,
+      minifierOpts,


nitpick 🐦 minifierOpts => minifyOpts ?

boopathi · 2017-08-17T13:41:24Z

I don't have permission to merge with failing checks. Can someone merge this?
Can someone give me push access to master to push release commits?

alexander-akait · 2017-08-17T13:44:37Z

@boopathi need somebody from https://github.com/orgs/webpack-contrib/people who have Owner 😞

michael-ciniawsky · 2017-08-17T14:13:55Z

Sry I can't, we are just grunt workers here :D Either set the status: Approved && pr: Merge ready labels or ping either (@)d3viant0ne (@)bebraw (@)TheLarkInn (@)SpaceK33z (@)jhnns (@)sokra to give you the appropiated access since you are lead maintainer here 😛

joshwiens · 2017-08-17T22:32:51Z

FYI - You have access to merge, it takes owner level access to override the checks.

boopathi · 2017-08-18T11:53:36Z

I still need permission to push to master for release commits. How are release commits pushed to master in webpack-contrib?

pi0 · 2017-08-18T11:56:08Z

@boopathi Just as a question, the package is released as 0.0.0 is it just for some internal reasons or not stable yet?

boopathi · 2017-08-18T11:58:14Z

The first version will be 0.2.0 - mostly keeping it the same version as babel-minify(0.2.0)

pi0 · 2017-08-18T12:08:36Z

OK so will wait for 0.2.0 before integrating with nuxt :)) Thanks for your hard work.

Rename babili to babel-minify

11fc287

michael-ciniawsky reviewed Aug 16, 2017

View reviewed changes

michael-ciniawsky added priority: 3 (required) pr_action: Review semver: Major status: Pending type: Refactor labels Aug 16, 2017

michael-ciniawsky changed the title ~~Rename babili to babel-minify~~ refactor: rename babili to babel-minify Aug 16, 2017

michael-ciniawsky changed the title ~~refactor: rename babili to babel-minify~~ refactor(src/index): rename babili to babel-minify Aug 16, 2017

michael-ciniawsky changed the title ~~refactor(src/index): rename babili to babel-minify~~ refactor: rename babili to babel-minify Aug 16, 2017

Simplify options

daea84b

michael-ciniawsky reviewed Aug 16, 2017

View reviewed changes

boopathi added 2 commits August 17, 2017 14:08

Disable some eslint rules; rename opt jsregex->test

66d6072

Update readme

d788353

michael-ciniawsky approved these changes Aug 17, 2017

View reviewed changes

michael-ciniawsky requested a review from alexander-akait August 17, 2017 13:18

alexander-akait approved these changes Aug 17, 2017

View reviewed changes

michael-ciniawsky added pr_action: Merge ready status: Approved and removed priority: 3 (required) pr_action: Review status: Pending labels Aug 17, 2017

joshwiens merged commit 3d55caf into master Aug 17, 2017

michael-ciniawsky deleted the rename-0 branch August 18, 2017 02:53

michael-ciniawsky removed the pr_action: Merge ready label Aug 18, 2017

edmorley mentioned this pull request Sep 26, 2017

Use babel-minify-webpack-plugin instead of babili-webpack-plugin neutrinojs/neutrino#330

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: rename `babili` to `babel-minify` #55

refactor: rename `babili` to `babel-minify` #55

boopathi commented Aug 16, 2017

codecov bot commented Aug 16, 2017 •

edited

Loading

pi0 commented Aug 16, 2017

michael-ciniawsky Aug 16, 2017

boopathi Aug 16, 2017

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

pi0 commented Aug 16, 2017

michael-ciniawsky commented Aug 16, 2017 •

edited

Loading

alexander-akait commented Aug 16, 2017

boopathi commented Aug 16, 2017 •

edited

Loading

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

michael-ciniawsky Aug 16, 2017

boopathi commented Aug 17, 2017

alexander-akait commented Aug 17, 2017

michael-ciniawsky commented Aug 17, 2017

joshwiens commented Aug 17, 2017

boopathi commented Aug 18, 2017

pi0 commented Aug 18, 2017

boopathi commented Aug 18, 2017

pi0 commented Aug 18, 2017

refactor: rename babili to babel-minify #55

refactor: rename babili to babel-minify #55

Conversation

boopathi commented Aug 16, 2017

codecov bot commented Aug 16, 2017 • edited Loading

Codecov Report

pi0 commented Aug 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pi0 commented Aug 16, 2017

michael-ciniawsky commented Aug 16, 2017 • edited Loading

alexander-akait commented Aug 16, 2017

boopathi commented Aug 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boopathi commented Aug 17, 2017

alexander-akait commented Aug 17, 2017

michael-ciniawsky commented Aug 17, 2017

joshwiens commented Aug 17, 2017

boopathi commented Aug 18, 2017

pi0 commented Aug 18, 2017

boopathi commented Aug 18, 2017

pi0 commented Aug 18, 2017

refactor: rename `babili` to `babel-minify` #55

refactor: rename `babili` to `babel-minify` #55

codecov bot commented Aug 16, 2017 •

edited

Loading

michael-ciniawsky commented Aug 16, 2017 •

edited

Loading

boopathi commented Aug 16, 2017 •

edited

Loading