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

Bundle size optimizations. #739

Closed
wants to merge 3 commits into from
Closed

Bundle size optimizations. #739

wants to merge 3 commits into from

Conversation

Herz3h
Copy link

@Herz3h Herz3h commented Aug 4, 2020

Replace date-fns by dayjs to reduce bundle size.
Replace lodash's each, assign, filter by native functions.
Add rollup-plugin-visualizer.

Before:
Capture d’écran 2020-08-04 à 13 37 36

After:
Capture d’écran 2020-08-04 à 14 02 03

Replace lodash's each, assign, filter by native functions.
Add rollup-plugin-visualizer.
@p0psicles
Copy link
Collaborator

You can't just swap out date-fns with dayjs. They both have different templating.
https://day.js.org/docs/en/parse/string-format vs https://date-fns.org/docs/format

Can't expect users to adjust their formats?

@p0psicles
Copy link
Collaborator

Can't we use es6 tree-shaking in stead?
https://date-fns.org/v2.0.0-alpha.7/docs/ECMAScript-Modules#tree-shaking

@Herz3h
Copy link
Author

Herz3h commented Aug 4, 2020

I only checked the parse format, which in both cases seem to accept ISO 8601 format by default. Hence the replacement. Will give tree-shaking a try tomorrow.

@Herz3h
Copy link
Author

Herz3h commented Aug 5, 2020

I put back date-fns in and tried date-fns/esm module, but not difference at all in bundle size.

Here is current bundle size:

image

@p0psicles
Copy link
Collaborator

p0psicles commented Aug 5, 2020

Yeah it's a build issue with poi/webpack egoist/poi#327

Well it should work with webpack 4. And we use the latest version of poi which uses webpack 4. So it as to be an issue with poi, or a config thing

@Herz3h
Copy link
Author

Herz3h commented Aug 5, 2020

I'm not familiar with poi/bili, but npm run bundle runs bili apparently, isn't that based on rollupJS ?

@p0psicles
Copy link
Collaborator

@xaksis ?

@xaksis
Copy link
Owner

xaksis commented Aug 9, 2020

hmm, curious. We do use bili for bundling which uses rollup internally. My memory is a bit fuzzy but this might be happening because of an issue with vue-rollup - egoist/bili#221 where we had to use inline bundling for esm file.
original issue on vgt - #569
I didn't get a chance to go back into it but that would be a good start to investigate, to see if we can revert the inline module bundling. basically remove this line: https://github.com/xaksis/vue-good-table/blob/master/bili.config.js#L34
and have the esm file not throw the error mentioned above.

@xaksis
Copy link
Owner

xaksis commented Aug 9, 2020

On another note, I think we can also replace lodash.foreach, lodash.filter, & lodash.assign with their es6 counterparts now and reduce the bundle size further. 🙏

edit: my bad, didn't notice that you already did this for lodash specific functions! Could you please resolve the conflicts so I can push this out next weekend? Thank you @Herz3h!

@Herz3h
Copy link
Author

Herz3h commented Nov 13, 2020

Fixed conflicts

@xaksis
Copy link
Owner

xaksis commented Feb 28, 2021

v2.21.6 contains these changes.

@xaksis xaksis closed this Feb 28, 2021
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.

None yet

3 participants