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

[Breaking Change][*] Chore: Use terser for optimizing cjs prod build #7047

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

etrepum
Copy link
Collaborator

@etrepum etrepum commented Jan 14, 2025

Breaking Change

This may cause some regression in the cjs build size or cause some other configuration related issue, particularly for www. I suspect that most people in OSS are using esm builds at this point, but some extra scrutiny should be used at Meta when evaluating this PR.

Description

The cjs prod build has used @ampproject/rollup-plugin-closure-compiler for some time, but that project is abandoned (last release Aug 2021) and causes hard to debug issues whenever you end up using syntax that is valid JS according to the rest of the toolchain but not supported by that abandoned project (e.g. property initializers in classes).

terser is already used for the lexical esm build which has been in production for some time now, it is currently the most popular tool for this purpose, and it is maintained on top of a current toolchain.

To investigate the difference between the two I audited our size-limit action since I knew there was an outstanding issue related to it (#6852). Turns out that issue is correct and the size-limit action hasn't done anything useful since we moved it to a pull_request_target trigger. Our size-limit action also only measured dev builds which is fast but not useful, so I changed it to prod. Unfortunately for (good) security reasons you can't really test these in a PR since the workflow code is guaranteed to come from main, so I had a few commits here testing the action with the pull_request trigger. We might still want to consider having a more abstract method for measuring size, e.g. publishing an artifact that a safer workflow trigger can measure.

Closes #6852

Test plan

Before

Get an acorn stack trace in prod builds that only includes the line number in some build artifact and not any real indication of where the error is (fortunately you can attach a debugger locally, but it's a pain) whenever modern JS syntax is used. Most recently I ran into this working on #7046 but this isn't the first time.

After

Syntax support is consistent across all builds, dev and prod.

Copy link

vercel bot commented Jan 14, 2025

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 Jan 14, 2025 5:21am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 5:21am

@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 Jan 14, 2025

This comment was marked as outdated.

@etrepum
Copy link
Collaborator Author

etrepum commented Jan 14, 2025

npm run size from this branch
  lexical - cjs
  Size:         29.69 kB with all dependencies, minified and brotlied
  Loading time: 580 ms   on slow 3G
  
  lexical - esm
  Size:         29.53 kB with all dependencies, minified and brotlied
  Loading time: 577 ms   on slow 3G
  
  @lexical/rich-text - cjs
  Size:         38.88 kB with all dependencies, minified and brotlied
  Loading time: 760 ms   on slow 3G
  
  @lexical/rich-text - esm
  Size:         31.71 kB with all dependencies, minified and brotlied
  Loading time: 620 ms   on slow 3G
  
  @lexical/plain-text - cjs
  Size:         37.38 kB with all dependencies, minified and brotlied
  Loading time: 731 ms   on slow 3G
  
  @lexical/plain-text - esm
  Size:         28.81 kB with all dependencies, minified and brotlied
  Loading time: 563 ms   on slow 3G
  
  @lexical/react - cjs
  Size:         40.8 kB with all dependencies, minified and brotlied
  Loading time: 797 ms  on slow 3G
  
  @lexical/react - esm
  Size:         32.98 kB with all dependencies, minified and brotlied
  Loading time: 645 ms   on slow 3G
npm run size from main
  lexical - cjs
  Size:         29.07 kB with all dependencies, minified and brotlied
  Loading time: 568 ms   on slow 3G
  
  lexical - esm
  Size:         29.53 kB with all dependencies, minified and brotlied
  Loading time: 577 ms   on slow 3G
  
  @lexical/rich-text - cjs
  Size:         38.3 kB with all dependencies, minified and brotlied
  Loading time: 749 ms  on slow 3G
  
  @lexical/rich-text - esm
  Size:         31.71 kB with all dependencies, minified and brotlied
  Loading time: 620 ms   on slow 3G
  
  @lexical/plain-text - cjs
  Size:         36.81 kB with all dependencies, minified and brotlied
  Loading time: 719 ms   on slow 3G
  
  @lexical/plain-text - esm
  Size:         28.81 kB with all dependencies, minified and brotlied
  Loading time: 563 ms   on slow 3G
  
  @lexical/react - cjs
  Size:         40.06 kB with all dependencies, minified and brotlied
  Loading time: 783 ms   on slow 3G
  
  @lexical/react - esm
  Size:         32.98 kB with all dependencies, minified and brotlied
  Loading time: 645 ms   on slow 3G

@etrepum
Copy link
Collaborator Author

etrepum commented Jan 14, 2025

All of the measurement differences are <1kb, e.g. lexical cjs prod went from 29.07 kB in main to 29.69 kB with terser (+ ~2.1%).

@potatowagon
Copy link
Contributor

Run npm run build-www job looks fine

This may cause some regression in the cjs build size or cause some other configuration related issue, particularly for www

would the only risk be wrongly reported build size?

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.

thanks for this upgrade!

@etrepum
Copy link
Collaborator Author

etrepum commented Jan 14, 2025

Run npm run build-www job looks fine

This may cause some regression in the cjs build size or cause some other configuration related issue, particularly for www

would the only risk be wrongly reported build size?

That’s the only known risk, I can’t know what else is going on in the www toolchain outside of lexical that could cause issues but it is very unlikely.

@etrepum etrepum added this pull request to the merge queue Jan 14, 2025
Merged via the queue into facebook:main with commit c8f27ed Jan 14, 2025
64 of 69 checks passed
@etrepum etrepum deleted the terser branch January 14, 2025 17:12
GermanJablo added a commit to GermanJablo/lexical that referenced this pull request Jan 30, 2025
commit a62a1a6
Author: James Fitzsimmons <[email protected]>
Date:   Thu Jan 30 19:13:35 2025 +1100

    [lexical-mark] Feature: include inline decorator nodes in marks (facebook#7086)

commit 881c7fe
Author: Bob Ippolito <[email protected]>
Date:   Thu Jan 30 00:13:00 2025 -0800

    [lexical-utils] Fix: Modify $reverseDfs to be a right-to-left variant of $dfs (facebook#7112)

commit ce2bb45
Author: Nigel Gutzmann <[email protected]>
Date:   Wed Jan 29 14:41:12 2025 -0800

    [lexical-utils] Feature: add reverse dfs iterator (facebook#7107)

commit 3a140d2
Author: mohammed shaheer kp <[email protected]>
Date:   Tue Jan 28 06:19:45 2025 +0530

    [lexical-playground] Bug Fix: Ensure Delete Node handles all node types (facebook#7096)

    Co-authored-by: shaheerkpzaigo <[email protected]>

commit 8e2ede2
Author: Adam Pugh <[email protected]>
Date:   Mon Jan 27 18:49:38 2025 -0600

    Listeners Lexical: 3 updates to spelling and grammar - Update listeners.md  (facebook#7100)

commit 9fcc494
Author: Adam Pugh <[email protected]>
Date:   Mon Jan 27 18:49:34 2025 -0600

    Lexical Docs: 2 updates to spelling README.md (facebook#7102)

commit 946a6df
Author: Adam Pugh <[email protected]>
Date:   Mon Jan 27 18:49:29 2025 -0600

    Selection | Lexical: 1 Spelling Update Update selection.md (facebook#7103)

commit ce93ea6
Author: Adam Pugh <[email protected]>
Date:   Mon Jan 27 18:49:25 2025 -0600

    Creating a React Plugin: 1 Grammar Update - Update create_plugin.md (facebook#7104)

commit ed29d89
Author: Adam Pugh <[email protected]>
Date:   Mon Jan 27 18:49:21 2025 -0600

    Working with DOM Events: 2 Spelling and Grammar Updates Update dom-ev… (facebook#7105)

commit 212b70f
Author: James Fitzsimmons <[email protected]>
Date:   Mon Jan 27 08:48:09 2025 +1100

    [lexical-yjs] Bug Fix: handle text node being split by Yjs redo (facebook#7098)

commit 6a98a47
Author: Torleif Berger <[email protected]>
Date:   Fri Jan 24 21:46:45 2025 +0100

    [lexical-react] Bug Fix: Import `JSX` type from React to prevent "Cannot find namespace 'JSX'"-error when type-checking with React 19 (facebook#7080)

commit f8e5968
Author: Tetsuya <[email protected]>
Date:   Sat Jan 25 04:06:57 2025 +0800

    [lexical] Chore: Rename variable and add comments for Safari compositing workaround (facebook#7092)

commit 81c9ab6
Author: Mateo Vuković <[email protected]>
Date:   Fri Jan 24 18:44:05 2025 +0100

    Fix: Use already defined RegisteredNodes type (facebook#7085)

commit 63958a2
Author: Sherry <[email protected]>
Date:   Tue Jan 21 18:15:21 2025 +0800

    [playground] Bug fix: prevent growing whitespaces in markdown table toggle (facebook#7041)

    Co-authored-by: Bob Ippolito <[email protected]>

commit d9f9924
Author: Sherry <[email protected]>
Date:   Tue Jan 21 14:58:08 2025 +0800

    Unrevert [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) facebook#7023 (facebook#7068)

commit 92fa0a3
Author: mohammed shaheer kp <[email protected]>
Date:   Tue Jan 21 06:23:24 2025 +0530

    [lexical-playground] plugins TableOfContent Scroll smooth behaviour A… (facebook#7069)

commit 2e4a63e
Author: Ivaylo Pavlov <[email protected]>
Date:   Mon Jan 20 02:37:34 2025 +0000

    [lexical-playground] Fix Columns Layout Item Overflow (facebook#7066)

commit d319b07
Author: Bob Ippolito <[email protected]>
Date:   Sun Jan 19 14:45:41 2025 -0800

    Change fork modules to use production only when NODE_ENV explicitly set to production (facebook#7065)

commit 46c9c2f
Author: CityHunter <[email protected]>
Date:   Sat Jan 18 13:00:38 2025 +0800

    [lexical] Bug Fix: In the Safari browser, during the compositing event process, the delete key exhibits unexpected behavior. (facebook#7061)

    Co-authored-by: 涂博闻 <[email protected]>

commit 92a1cd7
Author: Violet Rosenzweig <[email protected]>
Date:   Thu Jan 16 18:44:11 2025 -0500

    docs: Change "here" link to more descriptive text (facebook#7058)

commit f6377a3
Author: Aman Harwara <[email protected]>
Date:   Fri Jan 17 02:08:17 2025 +0530

    [lexical-table] Bug Fix: Prevent error if pasted table has empty row (facebook#7057)

commit 0835029
Author: Aman Harwara <[email protected]>
Date:   Fri Jan 17 00:18:08 2025 +0530

    [lexical-list] Bug Fix: Prevent error when calling formatList when selection is at root (facebook#6994)

commit 940435d
Author: Brayden <[email protected]>
Date:   Wed Jan 15 16:10:01 2025 -0800

    fix: iOS Autocorrect strips formatting by reporting wrong dataType (facebook#5789)

    Co-authored-by: Bob Ippolito <[email protected]>

commit 136a565
Author: Aman Harwara <[email protected]>
Date:   Thu Jan 16 04:48:32 2025 +0530

    [lexical-yjs] Feature: Allow passing in custom `syncCursorPositions` function to collab hook (facebook#7053)

commit 415c576
Author: Maksim Horbachevsky <[email protected]>
Date:   Wed Jan 15 18:18:03 2025 -0500

    fix: triple click around inline elements (links) (facebook#7055)

commit a3ef4f3
Author: Ivaylo Pavlov <[email protected]>
Date:   Wed Jan 15 23:15:39 2025 +0000

    [lexical-table] Support table alignment (facebook#7044)

commit 29d733c
Author: Sherry <[email protected]>
Date:   Wed Jan 15 21:50:07 2025 +0800

    Revert [Breaking Change][lexical] Bug Fix: Commit updates on editorSetRootElement(null) (facebook#7023) (facebook#7052)

commit 65ce66a
Author: Bob Ippolito <[email protected]>
Date:   Tue Jan 14 14:57:54 2025 -0800

    [lexical] Bug Fix: Normalize selection after applyDOMRange to account for Firefox differences (facebook#7050)

commit bbc07af
Author: Bob Ippolito <[email protected]>
Date:   Tue Jan 14 08:55:46 2025 -0800

    [*] Bug Fix: Use GITHUB_OUTPUT instead of GITHUB_ENV for size-limit action (facebook#7051)

commit c8f27ed
Author: Bob Ippolito <[email protected]>
Date:   Tue Jan 14 06:36:13 2025 -0800

    [Breaking Change][*] Chore: Use terser for optimizing cjs prod build (facebook#7047)

commit 8bd22d5
Author: Bob Ippolito <[email protected]>
Date:   Mon Jan 13 07:09:31 2025 -0800

    [lexical] Bug Fix: Handle MutationObserver/input event re-ordering when using contentEditable inside of an iframe (facebook#7045)

commit 930629c
Author: Ivaylo Pavlov <[email protected]>
Date:   Sat Jan 11 06:03:30 2025 +0000

    Clean up nested editor update (facebook#7039)

commit bd874a3
Author: Bob Ippolito <[email protected]>
Date:   Fri Jan 10 15:23:54 2025 -0800

    [Breaking Change][lexical][lexical-selection][lexical-list] Bug Fix: Fix infinite loop when splitting invalid ListItemNode (facebook#7037)

commit 541fa43
Author: Bob Ippolito <[email protected]>
Date:   Thu Jan 9 12:42:23 2025 -0800

    v0.23.1 (facebook#7035)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit d7abafd
Author: Bob Ippolito <[email protected]>
Date:   Thu Jan 9 08:33:12 2025 -0800

    [Breaking Change][lexical] Bug Fix: Commit updates on editor.setRootElement(null) (facebook#7023)

commit 6add515
Author: Bob Ippolito <[email protected]>
Date:   Wed Jan 8 17:27:15 2025 -0800

    [lexical] Fix TabNode deserialization regression  (facebook#7031)

commit 33e3677
Author: Maksim Horbachevsky <[email protected]>
Date:   Wed Jan 8 14:59:03 2025 -0500

    [lexical-react] Feature: Merge TabIndentionPlugin and ListMaxIndentLevelPlugin plugins (facebook#7018)

commit 7de86e4
Author: James Fitzsimmons <[email protected]>
Date:   Wed Jan 8 09:45:25 2025 +1100

    [lexical-mark] Bug Fix: reverse ternary in MarkNode.addID (facebook#7020)

commit 7961130
Author: Bob Ippolito <[email protected]>
Date:   Sun Jan 5 13:55:25 2025 -0800

    v0.23.0 (facebook#7017)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit 2b4252d
Author: Aman Harwara <[email protected]>
Date:   Sat Jan 4 11:31:19 2025 +0530

    [lexical-yjs] Feature: Expose function to get anchor and focus nodes for given user awareness state (facebook#6942)

commit 8100d6d
Author: Ivaylo Pavlov <[email protected]>
Date:   Sat Jan 4 01:12:04 2025 +0000

    [lexical-playground] Fix table hover actions button position (facebook#7011)

commit bd1ef2a
Author: Bob Ippolito <[email protected]>
Date:   Fri Jan 3 14:25:31 2025 -0800

    [lexical] Bug Fix: Fix registerNodeTransform regression introduced in facebook#6894 (facebook#7016)

commit 85c08b6
Author: Christian Grøngaard <[email protected]>
Date:   Thu Jan 2 00:20:20 2025 +0100

    [lexical-playground] Refactor: switch headings test file names (facebook#7008)

commit 7c21d4f
Author: Bob Ippolito <[email protected]>
Date:   Wed Jan 1 12:48:12 2025 -0800

    [Breaking Change][lexical] Feature: Add updateFromJSON and move more textFormat/textStyle to ElementNode (facebook#6970)

commit aaa9009
Author: Bob Ippolito <[email protected]>
Date:   Wed Jan 1 07:50:39 2025 -0800

    [lexical] Bug Fix: Fix getNodes over-selection (facebook#7006)

commit 803391d
Author: Sherry <[email protected]>
Date:   Tue Dec 31 11:26:17 2024 +0800

    [__test__] npm upgrade astro (facebook#7001)

commit 684352b
Author: Christian Grøngaard <[email protected]>
Date:   Mon Dec 30 05:12:45 2024 +0100

    Documentation: Fix typo "nest nest"->"nest" in README.md (facebook#7000)

    Co-authored-by: Bob Ippolito <[email protected]>

commit 27b75cc
Author: Sherry <[email protected]>
Date:   Fri Dec 27 11:06:29 2024 +0800

    [__tests__] npm upgrade next (facebook#6996)

commit 05ddbcc
Author: Simon <[email protected]>
Date:   Thu Dec 26 03:37:50 2024 +0100

    [lexical] Bug Fix: Flow is missing some variables and functions (facebook#6977)

commit e79c946
Author: Sherry <[email protected]>
Date:   Tue Dec 24 09:54:46 2024 +0800

    v0.22.0 (facebook#6993)

    Co-authored-by: Lexical GitHub Actions Bot <>

commit c415f7a
Author: Sam Zhou <[email protected]>
Date:   Mon Dec 23 10:31:36 2024 -0800

    [lexical-react] Refactor: Replace `React$MixedElement` and `React$Node` with `React.MixedElement` and `React.Node` (facebook#6984)

commit c844a4d
Author: Sherry <[email protected]>
Date:   Tue Dec 24 02:30:52 2024 +0800

    [lexical] Fix flow error: change this to any (facebook#6992)

commit 6190033
Author: Germán Jabloñski <[email protected]>
Date:   Mon Dec 23 05:19:27 2024 -0300

    Refactor: exportJSON (facebook#6983)

commit e0dafb8
Author: Germán Jabloñski <[email protected]>
Date:   Sat Dec 21 13:59:01 2024 -0300

    feature: expose forEachSelectedTextNode (facebook#6981)

    Co-authored-by: Bob Ippolito <[email protected]>

commit 23715f5
Author: Alex <[email protected]>
Date:   Fri Dec 20 18:23:27 2024 +0300

    [lexical][lexical-table] Bug fix: TablePlugin:  - check is current selection in target table node (facebook#6979)

    Co-authored-by: alazarev <[email protected]>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: CI Workflow 'Bundles - size-limit' runs onmain branch, not PR
4 participants