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

NPM package size is huge #56

Open
anuveyatsu opened this issue Jul 5, 2019 · 8 comments
Open

NPM package size is huge #56

anuveyatsu opened this issue Jul 5, 2019 · 8 comments

Comments

@anuveyatsu
Copy link
Member

anuveyatsu commented Jul 5, 2019

The package increases bundle size significantly, please, check:
https://bundlephobia.com/[email protected]

image

@rufuspollock
Copy link
Contributor

nearly 50% is xlsx which is not even essential IMO. I wonder if all "readers" other than CSV should be outside of core and only built optionally (we could do 2 builds: a full build and a lean build).

Also why do we need moment? (and is iconv-lite necessary?)

@anuveyatsu
Copy link
Member Author

anuveyatsu commented Sep 3, 2019

@rufuspollock this is one of the reasons for data explorer's size. cc/ @starsinmypockets

@starsinmypockets
Copy link

  • Removing xls seems like a no-brainer
  • I like the idea of having pluggable or configurable builds
  • dayjs replaces moment, is 2kb and has 23k ⭐️ (i've never used it)

@rufuspollock
Copy link
Contributor

@starsinmypockets can you have a look at this and estimate.

I also think iconv-lite is not need most of the time.

@starsinmypockets
Copy link

@rufuspollock

I would estimate:

  • 2 hrs to strip xls (or see lazy-loading below)
  • 2 hrs to replace moment
  • .5 - 1d implement a lazy-loading scheme (load if needed only -- I think there's some risk here / need further analysis)
  • stripping iconv-lite we should wait on it -- we use it to decode non-utf8 encoded csv's -- a robust solution here will probably save us all loads of hair-pulling and is worth the weight. Should be able to lazy-load it also as per lazy-loading scheme (tbd)

@rufuspollock
Copy link
Contributor

OK, i think we should think about this in the bigger picture of data explorer first.

@anuveyatsu
Copy link
Member Author

anuveyatsu commented Sep 17, 2019

xlsx seems huge as it has both reader and writer for xlsx/xls. I guess we need only xlsx/xls reader so we could replace it with, e.g., xlsx-extractor which is 4x smaller (163kb vs 646kb).

@anuveyatsu
Copy link
Member Author

@starsinmypockets I think implementing lazy loading would make sense in data explorer but not in data.js.

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

No branches or pull requests

3 participants