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

Do not build json3 module with Webpack #977

Merged
merged 1 commit into from Mar 30, 2017
Merged

Do not build json3 module with Webpack #977

merged 1 commit into from Mar 30, 2017

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Mar 19, 2017

Both of these are only used by socket.io and it contributes an unnecessary amount of size in the final vendor chunk. It removes 23kB from the final vendor chunk in production build.

@xPaw xPaw added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Mar 19, 2017
@xPaw xPaw added this to the 2.3.0 milestone Mar 19, 2017
@astorije astorije self-requested a review March 27, 2017 23:06
@astorije astorije removed their assignment Mar 27, 2017
astorije
astorije requested changes Mar 27, 2017
View reviewed changes
package.json
@@ -17,7 +17,7 @@
"start-dev": "npm-run-all --parallel watch start",
"build": "npm-run-all build:*",
"build:font-awesome": "node scripts/build-fontawesome.js",
"build:webpack": "webpack",
"build:webpack": "webpack --progress",
Copy link
Member

Choose a reason for hiding this comment

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

Is this a side-effect of the rest of this PR, or just something you've added along the way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Along the way.

package.json Outdated
@@ -75,6 +75,7 @@
"socket.io-client": "1.7.3",
"stylelint": "7.9.0",
"urijs": "1.18.9",
"webpack": "2.2.1"
"webpack": "2.2.1",
"webpack-remove-debug": "0.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

This comes from https://github.com/johngodley/webpack-remove-debug.
1 commit only, 0 stars/forks, pretty much no downloads... Shouldn't we inline the 6 lines of code of this project directly into our Webpack config, instead?

Also, I'm curious of such little usage of this, what do other projects do, they just leave the debug stuff in there?
Finally, do we want to remove this from both production and development environment, or is there an interest in keeping this in dev mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes i was thinking about inlining it.

Copy link
Member

Choose a reason for hiding this comment

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

I tried to inline it (using a function) but it looks like we have to make it an internal module in its own file and set it as loader: "./file". That sucks. Anyone knows a way we could use to simply define this one-line function at the top of the webpack config and call that function from the loader?!

Copy link
Member Author

Choose a reason for hiding this comment

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

Could just exclude it and only keep the json3 trick.

@astorije astorije mentioned this pull request Mar 28, 2017
Closed
@astorije astorije changed the title Do not build json3 and debug modules Do not build Webpack's json3 module Mar 30, 2017
@astorije astorije changed the title Do not build Webpack's json3 module Do not build json3 module with Webpack Mar 30, 2017
@xPaw
Copy link
Member Author

xPaw commented Mar 30, 2017

👍

astorije
astorije approved these changes Mar 30, 2017
View reviewed changes
Copy link
Member

@astorije astorije left a comment

Choose a reason for hiding this comment

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

Tested locally and it works OK. It does save a small chunk so nice! In dev env, vendor package went from 799kb to 755kb, in prod env it went from 259kb to 251kb (looked at Travis).

@astorije astorije mentioned this pull request Mar 30, 2017
Closed
@astorije
Copy link
Member

I moved to debug stuff to #993 for reference.

Merging this with one review as it's a very simple change and comes from a maintainer.

@astorije astorije merged commit d5d3cb6 into master Mar 30, 2017
@astorije astorije deleted the xpaw/reduce-vendor branch March 30, 2017 05:27
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
@astorije
Merge pull request thelounge#977 from thelounge/xpaw/reduce-vendor
42985c8 
Do not build json3 module with Webpack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astorije astorije astorije approved these changes

Assignees
No one assigned
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Milestone
2.3.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@xPaw @astorije