Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

perf: bundle size #915

Merged
merged 21 commits into from
Mar 15, 2019
Merged

perf: bundle size #915

merged 21 commits into from
Mar 15, 2019

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented Dec 18, 2018

Before:
808 modules
size:
3 627 606 B
1 308 055 B minified
346.3 KB minified/gzip

bundle analyzer https://bundle-size.netlify.com/http-before.html
screenshot 2018-12-18 at 16 23 02

After:
493 modules
size:
1 919 990 B
779 949 B minified
218.06 KB minified/gzip

bundle analyzer https://bundle-size.netlify.com/http-after.html
screenshot 2018-12-18 at 16 22 14

-37.0314756% improvement in size
-38.985148515 % improvement in modules to load

resolves #828

@ghost ghost assigned hugomrdias Dec 18, 2018
@ghost ghost added the in progress label Dec 18, 2018
@hugomrdias hugomrdias changed the title Fix/bundle size [WIP] Fix/bundle size Dec 18, 2018
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Thanks @hugomrdias this LGTM 🚀, just one comment that needs to be addressed.

@@ -68,7 +68,6 @@ function requireCommands () {
cmds.util = (send, config) => {
return {
getEndpointConfig: require('../util/get-endpoint-config')(config),
crypto: require('libp2p-crypto'),
Copy link
Contributor

Choose a reason for hiding this comment

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

README needs to be updated with this change and we need appropriate "BREAKING CHANGE" sections in the commits for this (as well as the switch to bignumber.js) so that they are reflected in the changelog when we release.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hugomrdias we need to do this please ^^

@alanshaw alanshaw changed the title [WIP] Fix/bundle size [WIP] perf: bundle size Jan 4, 2019
@alanshaw
Copy link
Contributor

alanshaw commented Feb 4, 2019

Note: #937 can be reverted after this is merged.

@hugomrdias hugomrdias changed the title [WIP] perf: bundle size perf: bundle size Feb 27, 2019
@hugomrdias
Copy link
Contributor Author

hugomrdias commented Feb 27, 2019

This PR is ready to merge. We still have readable-stream duplication, but it will be fixed soon with deps updates like bl and others.

Tests are a bit flaky had to restart a few times to get everything green, i have some follow ups to improve this.

@alanshaw
Copy link
Contributor

Is this ready? There's still github deps in the package.json

.travis.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@hugomrdias
Copy link
Contributor Author

i manage to improve some dependencies, bl and tar-stream got released today

we still have concat-stream, ndjson and stream-http

ndjson will probably be very difficult to get an upstream release
concat-stream will probably get released, i will create an issue to keep track
stream-http will be transformed in its own module to serve our needs better

@hugomrdias
Copy link
Contributor Author

@alanshaw i think we only need the breaking change msg about crypto and bignumber

Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Crypto breaking change already happened in 30.0.0 😀 and bignumber.js in 29.0.0.

@alanshaw alanshaw merged commit 87dff04 into master Mar 15, 2019
@ghost ghost removed the in progress label Mar 15, 2019
@hugomrdias hugomrdias deleted the fix/bundle-size branch March 15, 2019 09:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't resolve 'https'
2 participants