-
Notifications
You must be signed in to change notification settings - Fork 288
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
Enable Webpack tree shaking #352
Conversation
Okay this makes sense. So there are essentially two runtime contexts. One is the startup code (the server) and the other is the Next.js routes and associated code. You had to change everything in the server context, everything they import, and everything they export to use commonjs imports and exports, and then the Next.js context can be ES modules because Next imports them itself through webpack. Nice work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I tested it as well, and had no issues. Also fixed merge conflicts that just appeared a few minutes ago.
@willopez @rosshadden it looks like there are two lint errors that are being flagged in CI. |
They might have arisen from the merge conflicts. I'll take a look. |
Yeah my fault. I saw |
…commerce/reaction-next-starterkit into fix-enable-webpack-tree-shaking
Resolves #351
Impact: minor
Type: feature
Issue
Wepack tree shacking was not enabled due to the following setting being in our
.babelrc
,modules: commonjs
This PR removes that setting to effectively enable tree shaking(dead code elimination)
Solution
Follow NextJS' example when using a custom web server: https://github.com/zeit/next.js/blob/canary/examples/custom-server-express/server.js
Testing