Skip to content

use babel-plugin-lodash to reduce bundle size#28

Closed
nickav wants to merge 1 commit into
avocode:masterfrom
nickav:master
Closed

use babel-plugin-lodash to reduce bundle size#28
nickav wants to merge 1 commit into
avocode:masterfrom
nickav:master

Conversation

@nickav

@nickav nickav commented Apr 28, 2017

Copy link
Copy Markdown

fixes #24

@petrbrzek

Copy link
Copy Markdown
Member

@nickaversano Thanks for the PR.

Comment thread src/shortcut-manager.js

return shortcuts
const shortcuts = _.map(cursor, this._parseShortcutDescriptor)
return _.values(_.compact(_.flatten(shortcuts)))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need _.values for anything?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

not sure, I tried to copy all existing functions over. got this from this line

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nickaversano Yeah, here you have to call value as the chain of functions are lazily evaluated. So I think we don't need _.values in this PR.

@natew

natew commented May 17, 2017

Copy link
Copy Markdown

As a side note, what's funny is once you get a big enough application we basically have to manually revert this. Once you hit about 15 of the lodash individual pieces, you'll want to instruct webpack to force sub-modules to use the main lodash bundle rather than all the split ones, otherwise you end up with a bigger overall bundle because lodash split out ones have extra overhead.

@DimitryDushkin

Copy link
Copy Markdown

Will it merge?

@petrbrzek

Copy link
Copy Markdown
Member

We'll go with #40 instead. But thank you anyway.

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.

Too big

4 participants