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.

Sign up for GitHub

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

Jump to bottom

Enable source maps in webpack chunking + bundling process #3793

Merged
merged 13 commits into from Mar 6, 2018
Merged

Enable source maps in webpack chunking + bundling process #3793

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
11e4f3d
Removed combine-assets-plugin, since its very broken
ptomasroos Feb 13, 2018
7a9f16d
Bundle everything into app.js on production build
ptomasroos Feb 13, 2018
7082eb7
Clean up
ptomasroos Feb 13, 2018
086efe1
Merge branch 'canary' into enable-source-maps
ptomasroos Feb 13, 2018
479ca8f
Removed app.js from server routes
ptomasroos Feb 15, 2018
1caf81d
Renamed app.js -> main.js and removed commons from loading
ptomasroos Feb 15, 2018
0d82e34
Remove commons and react CommonChunks
ptomasroos Feb 15, 2018
23fcf6d
Removed the commons route
ptomasroos Feb 15, 2018
e2366e9
Killing the entire build-stats hack for app.js
ptomasroos Feb 15, 2018
87f5c5a
Removed unused md5-file package
ptomasroos Feb 15, 2018
f2a879d
Merge branch 'canary' into enable-source-maps
ptomasroos Feb 15, 2018
51230f9
Merge with canary and fix some tests.
arunoda Mar 6, 2018
b358c9e
Merge branch 'canary' into enable-source-maps
arunoda Mar 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 0 additions & 33 deletions server/build/plugins/combine-assets-plugin.js
View file
Open in desktop

This file was deleted.

36 changes: 28 additions & 8 deletions server/build/webpack.js
View file
Open in desktop
Expand Up @@ -6,7 +6,6 @@ import CaseSensitivePathPlugin from 'case-sensitive-paths-webpack-plugin'
import WriteFilePlugin from 'write-file-webpack-plugin'
import FriendlyErrorsWebpackPlugin from 'friendly-errors-webpack-plugin'
import {getPages} from './webpack/utils'
import CombineAssetsPlugin from './plugins/combine-assets-plugin'
import PagesPlugin from './plugins/pages-plugin'
import NextJsSsrImportPlugin from './plugins/nextjs-ssr-import'
import DynamicChunksPlugin from './plugins/dynamic-chunks-plugin'
Expand Down Expand Up @@ -248,15 +247,11 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer
new webpack.DefinePlugin({
'process.env.NODE_ENV': JSON.stringify(dev ? 'development' : 'production')
}),
!isServer && new CombineAssetsPlugin({
input: ['manifest.js', 'react.js', 'commons.js', 'main.js'],
output: 'app.js'
}),
!dev && new webpack.optimize.ModuleConcatenationPlugin(),
!isServer && new PagesPlugin(),
!isServer && new DynamicChunksPlugin(),
isServer && new NextJsSsrImportPlugin({ dir, dist: config.distDir }),
!isServer && new webpack.optimize.CommonsChunkPlugin({
dev && !isServer && new webpack.optimize.CommonsChunkPlugin({
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this completely. We are removing the react bundle too.
So, there's no need this in dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

name: `commons`,
filename: `commons.js`,
minChunks (module, count) {
Expand Down Expand Up @@ -287,7 +282,7 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer
return count >= totalPages * 0.5
}
}),
!isServer && new webpack.optimize.CommonsChunkPlugin({
dev && !isServer && new webpack.optimize.CommonsChunkPlugin({
Copy link
Member

Choose a reason for hiding this comment

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

This negates the react split bundle optimization with uglify. It might not be needed anymore because of the uglify optimizations we did though. cc @arunoda

Copy link
Contributor

Choose a reason for hiding this comment

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

@ptomasroos let's remove this as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

name: 'react',
filename: 'react.js',
minChunks (module, count) {
Expand All @@ -306,9 +301,34 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer
return false
}
}),
!isServer && new webpack.optimize.CommonsChunkPlugin({
dev && !isServer && new webpack.optimize.CommonsChunkPlugin({
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep this.
This helps in bundle rebuilding process.

name: 'manifest',
filename: 'manifest.js'
}),
!dev && !isServer && new webpack.optimize.CommonsChunkPlugin({
name: 'main.js',
filename: 'app.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep this as main.js.
And in the server/document.js we can change to use main.js instead of app.js for both production and dev.

For dev you may need to use manifest.js before main.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

minChunks (module, count) {
// react
if (module.resource && module.resource.includes(`${sep}react-dom${sep}`) && count >= 0) {
return true
}

if (module.resource && module.resource.includes(`${sep}react${sep}`) && count >= 0) {
return true
}
// react end

// commons
// If there are one or two pages, only move modules to common if they are
// used in all of the pages. Otherwise, move modules used in at-least
// 1/2 of the total pages into commons.
if (totalPages <= 2) {
return count >= totalPages
}
return count >= totalPages * 0.5
// commons end
}
})
].filter(Boolean)
}
Expand Down