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

Feature: Change size-limit action to measure something more meaningful than the core modules #6137

Closed
etrepum opened this issue May 18, 2024 · 3 comments
Assignees
Labels
enhancement Improvement over existing feature infra

Comments

@etrepum
Copy link
Collaborator

etrepum commented May 18, 2024

The size-limit workflow, as currently configured, has the following deficiencies:

  • It only reports changes, it does not enforce any hard limits
  • There is no documentation for what these limits should be, currently just vibes based(?) other than the intro docs which say "The core package of Lexical is only 22kb in file size (min+gzip)" which is no longer true, at least w/o further processing it's 30kb (see below)
  • It only measures the prod+cjs builds of three modules: lexical, @lexical/plain-text, and @lexical/rich-text
  • Notably this does not consider any relationship with dependencies that are meant to be external (yjs, y-websocket, react, react-dom, etc.), dependencies that are meant to be bundled (prism in @lexical/code), or anything that involves a significant code transform (other than dead-code elimination of __DEV__)
$ cat node_modules/lexical/Lexical.prod.js | gzip | wc -c
   30746

See also:

@etrepum etrepum added the enhancement Improvement over existing feature label May 18, 2024
@Sahejkm Sahejkm self-assigned this May 19, 2024
@etrepum
Copy link
Collaborator Author

etrepum commented May 20, 2024

It might also be worth thinking about removing the running time measurements because they are so incredibly noisy in this CI environment. See #6143 (comment) which made no change to any code that size-limit can see (the www build is not used in this scenario) but is reporting >20% changes to runtime on two of the three modules. I think errors of this magnitude makes the numbers counter-productive because people may spend time investigating phantom performance regressions.

@Sahejkm
Copy link
Contributor

Sahejkm commented Jun 7, 2024

Hi @etrepum do you think we can close this issue right now ? Or do we need to have hard limits of failing the build once it cross a threshold ? Or any other package you think we should include in the size check before closing this ?

@etrepum
Copy link
Collaborator Author

etrepum commented Jun 7, 2024

Yeah I think it's good enough, it would be nice if we had some guidelines around size but at least there is a bit more coverage now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement over existing feature infra
Projects
None yet
Development

No branches or pull requests

3 participants