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

Significantly smaller bundles #850

Merged
merged 4 commits into from
Jun 9, 2019
Merged

Significantly smaller bundles #850

merged 4 commits into from
Jun 9, 2019

Conversation

fabiospampinato
Copy link
Contributor

@fabiospampinato fabiospampinato commented Jun 4, 2019

Hey @knsv, I've just implemented some changes previously discussed aimed at reducing mermaid's bundled size, which is currently pretty significant.

  • I've replaced moment with moment-mini, which got rid of all the useless locale files. I'd like to switch to dayjs entirely, but currently moment is used to parse some relative date formats like 2d, I'm not sure if that's really useful or not though.

  • I've removed the src folder from the allowed paths being published to NPM.

  • I had first reimplemented some basic lodash methods in vanilla JS and then imported only the specific non trivial ones, which reduced the bundle size, but then I noticed that another version of lodash was getting loaded by other dependencies, so later I aligned the versions so that only 1 copy of lodash get loaded (I guess this wasn't happening before because of the lock file).

The unminified bundle size went from ~4.5MB to ~2.3MB, while the minified bundle size went from ~1.4MB to ~800KB. IMHO that's a pretty significant improvement. It could be much smaller if the other libraries in our dependency tree stopped requiring the entirety of lodash, I've opened a few issues about that in their repos, let's see.

Let me know if you need any modification.

Closes #843

@coveralls
Copy link

Pull Request Test Coverage Report for Build 765

  • 2 of 16 (12.5%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 54.764%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/diagrams/git/gitGraphAst.js 2 5 40.0%
src/diagrams/git/gitGraphRenderer.js 0 11 0.0%
Files with Coverage Reduction New Missed Lines %
src/diagrams/git/gitGraphAst.js 1 92.26%
Totals Coverage Status
Change from base Build 764: -0.07%
Covered Lines: 2102
Relevant Lines: 3808

💛 - Coveralls

@iamkun
Copy link

iamkun commented Jun 4, 2019

Hi @fabiospampinato, I'm the maintainer of Day.js that you mentioned above. If there's any I can do to help the optimization or upgrade Day.js accordingly, feel free to contact me.

@fabiospampinato
Copy link
Contributor Author

@iamkun thanks! I'm not 100% sure about why replacing moment with dayjs didn't simply just work for mermaid, I'll have to investigate this further, I'll let you know if I can find any inconsistencies between the two etc.

@knsv I think I may actually use this webpack plugin to squeeze some more bytes out of the bundles, turns out that across all our dependencies only the following lodash functions being used:

_.bind
_.chain
_.clone
_.cloneDeep
_.constant
_.defaults
_.each
_.filter
_.find
_.flatten
_.forEach
_.has
_.isArray
_.isEmpty
_.isFunction
_.isPlainObject
_.isUndefined
_.keys
_.last
_.map
_.mapValues
_.max
_.maxBy
_.merge
_.min
_.minBy
_.now
_.pick
_.range
_.reduce
_.size
_.sortBy
_.toPairs
_.transform
_.union
_.uniqueId
_.values
_.zipObject

So I guess there is still a significant room for improvement bundle-size-wise.

@fabiospampinato
Copy link
Contributor Author

Actually using lodash-webpack-plugin lead to some very marginal improvements, maybe I'm not using it correctly or something.

@knsv
Copy link
Collaborator

knsv commented Jun 4, 2019

Thanks Fabio. Will take alook at this and merge this weekend.

@knsv knsv merged commit ed3d155 into mermaid-js:master Jun 9, 2019
@eskatos
Copy link

eskatos commented Mar 29, 2021

Is it still possible to include/set a locale after this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smaller bundles
5 participants