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

Feature/webpack3 #2574

Merged
merged 20 commits into from Jul 25, 2017

Conversation

Projects
None yet
7 participants
@themre
Copy link
Contributor

commented Jun 20, 2017

Since Webpack 3 is officially released, I created PR with WP3 bump.

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2017

@shrynx

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2017

@themre Webpack 3's biggest feature being scopes, using ModuleConcatenationPlugin should be used. Also extract-text-plugin is noy yet compatible with webpack 3 webpack-contrib/extract-text-webpack-plugin#494

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2017

yeah, I know, I just wanted to add feature by feature and see where it fails. and obviously it fails only by bumping :)

Marko Trebizan
@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jun 22, 2017

No need to worry about AppVeyor for now, it's pretty flaky because many tools don't support Windows as well as their macOS/nix counterparts.

Just try to get Travis passing. 😄

Thanks a lot for looking into this.

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

thanks for the info! added ModuleConcatenationPlugin for when later. also noticed this:
webpack/webpack#5089

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

I don't think it's appveyor now. noticed this also when running locally: Module not found: Can't resolve 'source-map-loader' in 'C:\projects\create-react-app-a3khu'. Do we need to add source-map-loader into dependencies?

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jun 22, 2017

Yeah, be advised that the ModuleConcatenationPlugin is still highly experimental.

We'll need to keep a close eye on webpack issues and run tests on large projects to find any edge cases. It may be too buggy to ship immediately.

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jun 22, 2017

Is there any reason you included the source-map-loader commit?
I'd say it's safe to remove that commit for now. I'd like for this PR to focus on webpack 3 and leave #2355 to focus on source-map-loader.

Marko Trebizan
@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

strange, it merged that commit, but not package.json (which is missing import of source-map-loader). removed it for now.

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jun 22, 2017

Great! Thanks.

I see a warning:

Creating an optimized production build...
(node:4259) DeprecationWarning: Chunk.modules is deprecated. Use Chunk.getNumberOfModules/mapModules/forEachModule/containsModule instead.
Compiled successfully.

Would you mind taking a peek into this and identifying the offending module?

We need to compile a list of plugins that were broken and need updating by webpack 3.

Additionally, we need to compile a list of breaking changes in webpack@3 that may affect us.

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

extract-text-webpack-plugin.
webpack-contrib/extract-text-webpack-plugin#494
PR is here: webpack-contrib/extract-text-webpack-plugin#543
let's wait for it and then bump it.

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

@shrynx already mentioned it :) thanks!

Marko Trebizan added some commits Jul 7, 2017

Marko Trebizan
Marko Trebizan
Marko Trebizan
Marko Trebizan
@gaearon
Copy link
Member

left a comment

Thanks for working on it! Left a few comments.

@@ -254,6 +254,7 @@ module.exports = {
// Makes some environment variables available to the JS code, for example:
// if (process.env.NODE_ENV === 'development') { ... }. See `./env.js`.
new webpack.DefinePlugin(env.stringified),
new webpack.optimize.ModuleConcatenationPlugin(),

This comment has been minimized.

Copy link
@gaearon

gaearon Jul 8, 2017

Member

We shouldn't include this yet. It is too experimental.

@@ -293,6 +293,7 @@ module.exports = {
},
sourceMap: true,
}),
new webpack.optimize.ModuleConcatenationPlugin(),

This comment has been minimized.

Copy link
@gaearon
"url-loader": "0.5.8",
"webpack": "2.6.1",
"webpack-dev-server": "2.4.5",
"react-dev-utils": "^3.0.2",

This comment has been minimized.

Copy link
@gaearon

gaearon Jul 8, 2017

Member

This is our own module. Please don't bump it now. It is bumped during the release.

"webpack": "2.6.1",
"webpack-dev-server": "2.4.5",
"react-dev-utils": "^3.0.2",
"react-error-overlay": "^1.0.9",

This comment has been minimized.

Copy link
@gaearon

gaearon Jul 8, 2017

Member

Same.

@@ -48,13 +48,13 @@
"postcss-flexbugs-fixes": "3.0.0",

This comment has been minimized.

Copy link
@gaearon

gaearon Jul 8, 2017

Member

Shouldn't we need to bump ExtractTextPlugin?

Marko Trebizan and others added some commits Jul 8, 2017

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

ok, I removed ModuleConcatenationPlugin, downgraded react packages and bumped ETWP.

Marko Trebizan
@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2017

ETWP rc1 has been released. I updated it only to see if everything goes through so we can report if something else fails.

Marko Trebizan
@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2017

hm... it's failing, because:

./src/features/webpack/assets/abstract.json
(Emitted value instead of an instance of Error) ⚠️  JSON Loader
It seems you're using webpack >= v2.0.0, which includes native support for JSON.
Please remove this loader from webpack.config.js as it isn't needed anymore and
is deprecated. See the v1.0.0 -> v2.0.0 migration guide for more information
(https://webpack.js.org/guides/migrating/#json-loader-is-not-required-anymore)
C:/Users/appveyor/AppData/Local/Temp/1/tmp.bNXBiS0bLw/node_modules/test-integrity/package.json
(Emitted value instead of an instance of Error) ⚠️  JSON Loader

It seems initDOM.js (https://github.com/facebookincubator/create-react-app/blob/a171d930641f513279b1c7ceaab1fa9bbbbbb896/packages/react-scripts/fixtures/kitchensink/integration/initDOM.js) tries to load json-loader which isn't required anymore. How do we approach this?

@gaearon

This comment has been minimized.

Copy link
Member

commented Jul 22, 2017

Hmm I'm not sure where this is coming from 😞
Could this somehow be caused by this?

@gaearon

This comment has been minimized.

Copy link
Member

commented Jul 22, 2017

I filed an issue in webpack-contrib/json-loader#57.

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2017

great, thanks!

@d3viant0ne

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2017

@themre - I stripped the deprecation warning in json-loader@0.5.7 which is now available on npm.

To the issue in CompatibilityPlugin.js i'll take that issue up on slack shortly.

In the mean time, i am locking json-loader until we resolve the issue upstream. It was deprecated, should be deprecated by can't be until Webpack doesn't directly depend on it.

Marko Trebizan added some commits Jul 22, 2017

Marko Trebizan
Marko Trebizan
@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2017

seems everything is ok now.

@gaearon gaearon added this to the 1.0.11 milestone Jul 25, 2017

@gaearon gaearon merged commit bc82755 into facebook:master Jul 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gaearon

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

Sweet. Wanna send another PR to bump to webpack 3.4 that just came out?

@themre themre deleted the themre:feature/webpack3 branch Jul 25, 2017

@themre

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2017

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2017

Hi there! This change is out in react-scripts@1.0.11; please give it a go! Thanks.

JohnNilsson referenced this pull request in JohnNilsson/create-react-app-typescript Aug 9, 2017

Feature/webpack3 (#2574)
* Add source-map-loader to support source maps of dependencies

* merge with latest develop

* add ModuleConcatenationPlugin

* revert source-map-loader

* bump to 3.1 and update dev-server

* rebase to master

* rebase again

* bump webpack

* remove ModuleConcatenationPlugin, downgraded react packages, bump etwp

* ETWP rc1 only for testing

* bump ETWP and webpack 3

* bump WP3

* revert to 3.2.0

* bump sw-precache-webpack-plugin

* bump back to 3.3.0

* bump dev-server and manifest

JohnNilsson referenced this pull request in JohnNilsson/create-react-app-typescript Sep 9, 2017

Feature/webpack3 (#2574)
* Add source-map-loader to support source maps of dependencies

* merge with latest develop

* add ModuleConcatenationPlugin

* revert source-map-loader

* bump to 3.1 and update dev-server

* rebase to master

* rebase again

* bump webpack

* remove ModuleConcatenationPlugin, downgraded react packages, bump etwp

* ETWP rc1 only for testing

* bump ETWP and webpack 3

* bump WP3

* revert to 3.2.0

* bump sw-precache-webpack-plugin

* bump back to 3.3.0

* bump dev-server and manifest

Ayc0 pushed a commit to Ayc0/create-react-app that referenced this pull request Sep 18, 2017

Feature/webpack3 (facebook#2574)
* Add source-map-loader to support source maps of dependencies

* merge with latest develop

* add ModuleConcatenationPlugin

* revert source-map-loader

* bump to 3.1 and update dev-server

* rebase to master

* rebase again

* bump webpack

* remove ModuleConcatenationPlugin, downgraded react packages, bump etwp

* ETWP rc1 only for testing

* bump ETWP and webpack 3

* bump WP3

* revert to 3.2.0

* bump sw-precache-webpack-plugin

* bump back to 3.3.0

* bump dev-server and manifest

kasperpeulen added a commit to kasperpeulen/create-react-app that referenced this pull request Sep 24, 2017

Feature/webpack3 (facebook#2574)
* Add source-map-loader to support source maps of dependencies

* merge with latest develop

* add ModuleConcatenationPlugin

* revert source-map-loader

* bump to 3.1 and update dev-server

* rebase to master

* rebase again

* bump webpack

* remove ModuleConcatenationPlugin, downgraded react packages, bump etwp

* ETWP rc1 only for testing

* bump ETWP and webpack 3

* bump WP3

* revert to 3.2.0

* bump sw-precache-webpack-plugin

* bump back to 3.3.0

* bump dev-server and manifest

swengorschewski referenced this pull request in swengorschewski/cra-typescript-electron Oct 16, 2017

Feature/webpack3 (#2574)
* Add source-map-loader to support source maps of dependencies

* merge with latest develop

* add ModuleConcatenationPlugin

* revert source-map-loader

* bump to 3.1 and update dev-server

* rebase to master

* rebase again

* bump webpack

* remove ModuleConcatenationPlugin, downgraded react packages, bump etwp

* ETWP rc1 only for testing

* bump ETWP and webpack 3

* bump WP3

* revert to 3.2.0

* bump sw-precache-webpack-plugin

* bump back to 3.3.0

* bump dev-server and manifest

@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.
You can’t perform that action at this time.