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

[Lexical][Size-Checks] Measure both cjs/esm builds for regression checks #6219

Merged
merged 3 commits into from
May 31, 2024

Conversation

Sahejkm
Copy link
Contributor

@Sahejkm Sahejkm commented May 31, 2024

WHAT

follow up on #6137

  • Measure both cjs/esm build combinations for size checks
  • Also ignore size-limit file from triggering extended tests run

WHY

  • Current regression checks meaure only cjs checks, enhance to check both cjs/esm changes.
  • Not needed to run e2e tests for size-limit workflow updates

Test

sahejk@sahejk-mbp lexical % npx size-limit
✔ Adding to empty webpack project
✔ Running JS in headless Chrome
  
  lexical - cjs
  Size:         22.5 kB with all dependencies, minified and brotlied
  Loading time: 440 ms  on slow 3G
  
  lexical - esm
  Size:         22.97 kB with all dependencies, minified and brotlied
  Loading time: 449 ms   on slow 3G
  
  @lexical/rich-text - cjs
  Size:         33.25 kB with all dependencies, minified and brotlied
  Loading time: 650 ms   on slow 3G
  
  @lexical/rich-text - esm
  Size:         23.05 kB with all dependencies, minified and brotlied
  Loading time: 451 ms   on slow 3G
  
  @lexical/plain-text - cjs
  Size:         33.21 kB with all dependencies, minified and brotlied
  Loading time: 649 ms   on slow 3G
  
  @lexical/plain-text - esm
  Size:         225 B with all dependencies, minified and brotlied
  Loading time: 10 ms on slow 3G
  
  @lexical/react - cjs
  Size:         150.4 kB with all dependencies, minified and brotlied
  Loading time: 3 s      on slow 3G
  
  @lexical/react - esm
  Size:         83.86 kB with all dependencies, minified and brotlied
  Loading time: 1.7 s    on slow 3G
  
sahejk@sahejk-mbp lexical % 

Copy link

vercel bot commented May 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2024 5:48am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2024 5:48am

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 31, 2024
Copy link

github-actions bot commented May 31, 2024

size-limit report 📦

Path Size
lexical 23.97 KB (0%)
@lexical/rich-text 34.66 KB (0%)
@lexical/plain-text 34.65 KB (0%)
@lexical/react 149.31 KB (0%)

@etrepum
Copy link
Collaborator

etrepum commented May 31, 2024

Is there a reason to track dev size? It's not really clear what that would help with, since size is presumably not a primary concern for dev. It might be more useful to track the cjs and esm prod builds (probably not at Meta, but most other places are likely using esm or soon will be)

@Sahejkm Sahejkm changed the title [Lexical][Size-Checks] Measure both dev/prod builds for regression checks [Lexical][Size-Checks] Measure both cjs/esm builds for regression checks May 31, 2024
@Sahejkm
Copy link
Contributor Author

Sahejkm commented May 31, 2024

thanks for reviewing Bob, Makes sense to me. Updated. lmk WDYT now ?

@Sahejkm Sahejkm marked this pull request as ready for review May 31, 2024 05:01
Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Looks like it would work, but could be a bit simpler by using the exports for lookup instead of manipulating the paths directly

.size-limit.js Show resolved Hide resolved
.size-limit.js Outdated Show resolved Hide resolved
.size-limit.js Outdated Show resolved Hide resolved
.size-limit.js Outdated Show resolved Hide resolved
@Sahejkm
Copy link
Contributor Author

Sahejkm commented May 31, 2024

Resolved comments, thanks for rereview!

Copy link
Contributor

@potatowagon potatowagon left a comment

Choose a reason for hiding this comment

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

lgtm

@lexical-bot lexical-bot added the extended-tests Run extended e2e tests on a PR label May 31, 2024
@Sahejkm
Copy link
Contributor Author

Sahejkm commented May 31, 2024

test failures look unrelated, will check separately. Seems to impact other builds as well like https://github.com/facebook/lexical/pull/6216/checks

@Sahejkm Sahejkm added this pull request to the merge queue May 31, 2024
Merged via the queue into main with commit d14cc07 May 31, 2024
22 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants