Skip to content

Commit

Permalink
[ZEPPELIN-1850] Remove strip-loader (webpack)
Browse files Browse the repository at this point in the history
### What is this PR for?

Removed [strip-loader](https://issues.apache.org/jira/browse/ZEPPELIN-1850) which was added to remove `console.log` in production.
But we can achieve the same result by using uglify option. I missed it in #1805

- `strip-loader` has a bug as you can see in below (screenshot section)
- Deleting it help us to keep the package list simple
- Also using the same configuration for uglify both in webpack and grunt will ensure that vendor package have the consistent behavior when they are managed by webpack

Also, this should be merged before #1812 since we need to modify `yarn.lock` in #1812

### What type of PR is it?
[Bug Fix | Improvement]

### Todos

Done at once

### What is the Jira issue?

[ZEPPELIN-1850](https://issues.apache.org/jira/browse/ZEPPELIN-1850)

### How should this be tested?

1. Add this code to any js file in master branch and execute `npm run build` to lint. You will get an error

```
        console.log('editor isn\'t loaded yet, returning..');
```

2. Test the same command and code within this branch. It will work.

### Screenshots (if appropriate)

```
ERROR in ./src/app/notebook/paragraph/paragraph.controller.js
Module build failed: SyntaxError: Unexpected token, expected , (197:8)

  195 |             }
  196 |           }
> 197 |         });
      |         ^
  198 |       } else {
  199 |         BootstrapDialog.confirm({
  200 |           closable: true,

  ./src/index.js 53:0-59
Child html-webpack-plugin for "index.html":
        + 1 hidden modules
Running "useminPrepare:html" (useminPrepare) task
Configuration changed for concat, uglify, cssmin

Running "concurrent:dist" (concurrent) task

    Running "svgmin:dist" (svgmin) task
    Total saved: 0 B

    Done, without errors.

    Execution Time (2016-12-30 06:21:32 UTC)
    loading tasks  121ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 70%
    svgmin:dist     51ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 30%
    Total 172ms

    Running "copy:styles" (copy) task
    Copied 17 files

    Done, without errors.

    Execution Time (2016-12-30 06:21:32 UTC)
    loading tasks  127ms  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 77%
    copy:styles     38ms  ▇▇▇▇▇▇▇▇▇▇▇ 23%
    Total 165ms

Running "postcss:dist" (postcss) task
>> 17 processed stylesheets created.
>> No "concat" targets found.
Warning: Task "concat" failed. Use --force to continue.

Aborted due to warnings.

Execution Time (2016-12-30 06:21:30 UTC)
loading tasks       117ms  ▇▇▇▇ 6%
useminPrepare:html   22ms  ▇ 1%
concurrent:dist      1.5s  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ 80%
postcss:dist        252ms  ▇▇▇▇▇▇▇▇▇ 13%
Total 1.9s

npm ERR! Darwin 15.6.0
npm ERR! argv "/Users/lambda/.nvm/versions/node/v6.9.1/bin/node" "/Users/lambda/.nvm/versions/node/v6.9.1/bin/npm" "run" "build"
```

### Questions:
* Does the licenses files need update? - NO
* Is there breaking changes for older versions? - NO
* Does this needs documentation? - NO

Author: 1ambda <1amb4a@gmail.com>

Closes #1823 from 1ambda/ZEPPELIN-1850/remove-strip-loader-in-webpack and squashes the following commits:

137bcdd [1ambda] review: Fix invalid uglify plugin opt
3d3a581 [1ambda] fix: Remove strip-loader to avoid single quote error
  • Loading branch information
1ambda authored and Leemoonsoo committed Jan 4, 2017
1 parent a5b437d commit 49423eb
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
1 change: 0 additions & 1 deletion zeppelin-web/package.json
Expand Up @@ -66,7 +66,6 @@
"npm-run-all": "^3.1.2",
"postcss-loader": "^1.2.1",
"raw-loader": "^0.5.1",
"strip-loader": "^0.1.2",
"style-loader": "^0.13.1",
"time-grunt": "^0.3.1",
"webpack": "^1.14.0",
Expand Down
27 changes: 18 additions & 9 deletions zeppelin-web/webpack.config.js
Expand Up @@ -130,11 +130,6 @@ module.exports = function makeWebpackConfig () {
* This handles most of the magic responsible for converting modules
*/

var jsLoaders = ['ng-annotate', 'babel-loader'];
if (isProd) {
jsLoaders.push('strip-loader?strip[]=console.log');
}

// Initialize module
config.module = {
preLoaders: [],
Expand All @@ -144,7 +139,7 @@ module.exports = function makeWebpackConfig () {
// Transpile .js files using babel-loader
// Compiles ES6 and ES7 into ES5 code
test: /\.js$/,
loaders: jsLoaders,
loaders: ['ng-annotate', 'babel-loader'],
exclude: /(node_modules|bower_components)/,
}, {
// CSS LOADER
Expand Down Expand Up @@ -227,12 +222,26 @@ module.exports = function makeWebpackConfig () {

// Reference: http://webpack.github.io/docs/list-of-plugins.html#uglifyjsplugin
// Minify all javascript, switch loaders to minimizing mode
new webpack.optimize.UglifyJsPlugin(),
new webpack.optimize.UglifyJsPlugin({
mangle: { screw_ie8: true },
preserveComments: 'some',
compress: {
screw_ie8: true,
warnings: false,
sequences: true,
dead_code: true,
conditionals: true,
booleans: true,
unused: true,
if_return: true,
join_vars: true,
drop_console: true,
}
}),

// Copy assets from the public folder
// Reference: https://github.com/kevlened/copy-webpack-plugin
new CopyWebpackPlugin([
])
new CopyWebpackPlugin([])
)
} else {
config.plugins.push(new InsertLiveReloadPlugin())
Expand Down

0 comments on commit 49423eb

Please sign in to comment.