Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to webpack v2 #189

Closed
wants to merge 6 commits into from
Closed

Upgrade to webpack v2 #189

wants to merge 6 commits into from

Conversation

mxstbr
Copy link
Contributor

@mxstbr mxstbr commented Jul 25, 2016

Ref #183

This is a first step towards webpack 2. Everything builds and works just fine.

Build stats:

v1

4.0K    build/84287d09b8053c6fa598893b8910786a.svg
 28K    build/favicon.ico
4.0K    build/index.html
140K    build/main.c7a1d762a452b452a7e7.js
4.0K    build/main.7cbecfc47e1de8546c1a31e27e545145.css
4.0K    build/main.7cbecfc47e1de8546c1a31e27e545145.css.map
1.5M    build/main.c7a1d762a452b452a7e7.js.map

v2

4.0K    build/84287d09b8053c6fa598893b8910786a.svg
 28K    build/favicon.ico
4.0K    build/index.html
140K    build/main.3e41bc8b11563538fb8e.js
4.0K    build/main.5d2cacffab6444a5357d533f016e1b6d.css
4.0K    build/main.5d2cacffab6444a5357d533f016e1b6d.css.map

I think nothing is different because we have to not use babel-plugin-transform-es2015-modules-commonjs, which is what babel-preset-es2015-webpack does so we get the benefits of tree shaking.

Annoyingly we're using babel-preset-es2016, which doesn't have a babel-preset-es2016-webpack with babel-plugin-transform-es2015-modules-commonjs removed!

Would appreciate somebody forking babel-preset-es2016, removing that, publishing it and trying it with this PR!

I'm going to bed now, feel free to continue the work against the webpack-v2 branch!

@ghost ghost added the CLA Signed label Jul 25, 2016
@mxstbr mxstbr mentioned this pull request Jul 25, 2016
@@ -46,7 +46,7 @@
"eslint-plugin-import": "1.10.3",
"eslint-plugin-jsx-a11y": "2.0.1",
"eslint-plugin-react": "5.2.2",
"extract-text-webpack-plugin": "1.0.1",
"extract-text-webpack-plugin": "^2.0.0-beta.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed otherwise webpack fails with an error.

Copy link

Choose a reason for hiding this comment

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

I think we must remove caret because it's in beta and api can be changed.
I faced with this today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, that was an accident! Good catch!

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Annoyingly we're using babel-preset-es2016, which doesn't have a babel-preset-es2016-webpack with babel-plugin-transform-es2015-modules-commonjs removed!

Don’t worry about it, it is literally just exponentiation operator. All the meat is in 2016, and it doesn’t include 2015 by itself.

@@ -56,7 +56,7 @@
"rimraf": "2.5.3",
"style-loader": "0.13.1",
"url-loader": "0.5.7",
"webpack": "1.13.1",
"webpack": "^2.1.0-beta.20",
Copy link
Contributor

Choose a reason for hiding this comment

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

Also caret here

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

I’d never think it would be so easy 😄

@mxstbr
Copy link
Contributor Author

mxstbr commented Jul 25, 2016

Removed babel-preset-es2016 and built again, these are the stats:

4.0K    build/84287d09b8053c6fa598893b8910786a.svg
 28K    build/favicon.ico
4.0K    build/index.html
140K    build/main.3e41bc8b11563538fb8e.js
4.0K    build/main.5d2cacffab6444a5357d533f016e1b6d.css
4.0K    build/main.5d2cacffab6444a5357d533f016e1b6d.css.map

Same size for the main bundle, not 100% if that's expected or not anymore.

Also just realized that the .js.map is missing – is there any reason we're emitting source maps in production?

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Also just realized that the .js.map is missing – is there any reason we're emitting source maps in production?

To help debug issues, people can always delete them if they want to.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Removed babel-preset-es2016

Why? I meant that it’s orthogonal to commonjs support, not that it needs to be removed 😉

@sokra
Copy link

sokra commented Jul 25, 2016

Also just realized that the .js.map is missing

Pass sourceMap: true to the UglifyJsPlugin.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Same size for the main bundle, not 100% if that's expected or not anymore.

Sure, we wouldn’t get any benefits with vanilla React. I’m more interested in Webpack 2 because it’s easier to get fixes into it than backport them to 1.x.

@sokra
Copy link

sokra commented Jul 25, 2016

resolveLoader.root need to be changed too -> resolveLoader.modules

@@ -9,8 +9,7 @@

module.exports = {
presets: [
'babel-preset-es2015',
Copy link

Choose a reason for hiding this comment

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

Huh, it's weird that this isn't es2015-loose. I can't think of any good reason to not use es2015-loose (and correspondingly es2015-loose-native-modules instead of either es2015-native-modules or es2015-webpack) for production.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the difference?

Copy link

Choose a reason for hiding this comment

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

It makes all the transforms run in loose mode. It's like es2015-loose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's like es2015-loose.

Haha that's what I'm asking, what is loose mode?

Copy link

Choose a reason for hiding this comment

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

Loose mode generates code that is simpler, but less strictly compliant to spec. Compare output:

I'm not aware of any reason to not at least run your production build in loose mode. I'm also not aware of anybody who deliberately uses non-loose mode.

Copy link

Choose a reason for hiding this comment

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

Yeah that's it, some people would rather be compliant to the spec even though there's more output, etc

@mxstbr
Copy link
Contributor Author

mxstbr commented Jul 26, 2016

Why? I meant that it’s orthogonal to commonjs support, not that it needs to be removed 😉

I wish that was the case, but if I understand this correctly it isn't! It includes transform-modules-commonjs: https://github.com/hzoo/babel-preset-es2016/blob/4a6acc58215f07d4cb4377cfe1c344f79bae53c2/index.js#L22

@mxstbr
Copy link
Contributor Author

mxstbr commented Jul 26, 2016

Well, Node 4 with npm 2 fails because there's lots of invalid peerDependencies. Let me see if I can figure that out…

EDIT

Wait, what am I missing here, doesn't webpack@2.1.0-beta.20 satisfy all of those peerDeps?

npm ERR! peerinvalid The package webpack@2.1.0-beta.20 does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer babel-loader@6.2.4 wants webpack@1 || ^2.1.0-beta
npm ERR! peerinvalid Peer extract-text-webpack-plugin@2.0.0-beta.3 wants webpack@^2.1.0-beta
npm ERR! peerinvalid Peer html-webpack-plugin@2.22.0 wants webpack@*
npm ERR! peerinvalid Peer webpack-dev-server@1.14.1 wants webpack@>=1.3.0 <3

@taion
Copy link

taion commented Jul 26, 2016

You have the wrong version of webpack-dev-server

@taion
Copy link

taion commented Jul 26, 2016

aka npm error messages are useless

@vladshcherbin
Copy link

Try using es2015-loose-native-modules for babel, works great with webpack 2 and is dead simple.

@hzoo
Copy link

hzoo commented Jul 26, 2016

@mxstbr that repo is the old one, before I asked to move it to babel. es2016 just includes the exponential operator https://github.com/babel/babel/tree/master/packages/babel-preset-es2016

@ghost ghost added the CLA Signed label Jul 26, 2016
@insin insin mentioned this pull request Jul 26, 2016
@taion
Copy link

taion commented Jul 26, 2016

I wish it were named es2015-native-modules-loose ._. the name feels so awkward

@hzoo
Copy link

hzoo commented Jul 26, 2016

@taion What we really need to do is just merge babel/babel#3331 it again if it makes sense) @loganfsmyth - then everyone can just use es2015.

@ghost ghost added the CLA Signed label Jul 26, 2016
@taion
Copy link

taion commented Jul 26, 2016

@hzoo That'd be fantastic if it could happen. It's just painful to set up Babel to produce both a CJS and an ES module build right now.

@gaearon
Copy link
Contributor

gaearon commented Jul 26, 2016

Update: let’s keep it open but hold it off until Webpack 2 is out of beta. Thanks for your effort!
Why: #183 (comment).

@gaearon
Copy link
Contributor

gaearon commented Aug 2, 2016

Meh, keeping it open doesn’t help anyone. I really appreciate your work on this @mxstbr, and I’ll dig it up when Webpack 2 is stable / more official. Thanks!

@gaearon gaearon closed this Aug 2, 2016
@mxstbr mxstbr deleted the webpack-v2 branch August 2, 2016 12:54
@mxstbr
Copy link
Contributor Author

mxstbr commented Aug 2, 2016

Yeah I was wondering about that 😁

@axelson
Copy link

axelson commented Feb 23, 2017

FYI: #1291 was merged instead of this

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants