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

Split non-calendar code into modular files #479

Merged
merged 17 commits into from
Nov 4, 2024
Merged

Conversation

SLaks
Copy link
Contributor

@SLaks SLaks commented Nov 3, 2024

I think this isn't a breaking change.

This PR fixes most of #475:

  • Splits most APIs into separate files that can be imported directly
  • Reconfigures the ES modules output to emit individual files instead of a single giant file (to allow downstream bundlers to entirely ignore unused files).
    • This is necessary because tree shaking alone isn't good enough (I think due to side-effects)
  • Sets up a few demo files to show how each commit in the PR affects downstream bundle size
  • Documents most of this in the readme

SLaks added 15 commits November 3, 2024 14:18
Windows' cmd doesn't expand globs, so I explicitly list every file instead
And record their sizes on build
This relies on downstream applications to decide how/which/whether to include polyfills, saving ~50KB.

And remove polyfills entirely from size-demo
…single bundle.

This makes tree shaking much better.

This is a breaking change, but I can fix that by adding a forwarder.
Consuming applications will already install dependencies, so there is no point in bundling them.  This probably also helps resolve conflicts/duplication, where an application may have ended up importing its installed copy of `HDate` as separate from the bundled `HDate`.

 - This reduces index.cjs from 470KB to 177KB
 - This fixes the size increase from the previous commit, which had included Babel polyfills in the ES bundle
This demo file was the same 404KB before
@SLaks
Copy link
Contributor Author

SLaks commented Nov 3, 2024

This is not quite ready yet (I'm still testing & fixing leyning), but can you please take a look and let me know what you think?

This only has modest savings, because most of the code comes from this very function.

This is not worth splitting; instead, we should redo the API first.
Copy link

sonarcloud bot commented Nov 4, 2024

@mjradwin mjradwin merged commit 5707fc8 into hebcal:main Nov 4, 2024
4 of 8 checks passed
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.

2 participants