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

feat: pre-compile ui and richtext-lexical with react compiler #7688

Merged
merged 48 commits into from
Aug 19, 2024

Conversation

AlessioGr
Copy link
Member

@AlessioGr AlessioGr commented Aug 15, 2024

This noticeably improves performance in the admin panel, for example when there are multiple richtext editors on one page (& likely performance in other areas too, though I mainly tested rich text).

The babel plugin currently only optimizes files with a 'use client' directive at the top - thus we have to make sure to add use client wherever possible, even if it's imported by a parent client component.

There's one single component that broke when it was compiled using the React compiler (it stopped being reactive and failed one of our admin e2e tests): 150808f opting out of it completely fixed that issue

Fixes #7366. Canary: 3.0.0-canary.79e92af

test/package.json Outdated Show resolved Hide resolved
packages/richtext-lexical/babel.config.cjs Outdated Show resolved Hide resolved
@poteto
Copy link

poteto commented Aug 19, 2024

For context we've been using the compiler in prod on many Meta apps for several months now without any major issues. The issues we have seen usually involve some sketchy React pattern (that the compiler can't always detect statically) that results in unexpected behavior. However in those cases once the offending component/hook(s) are found it's trivial to disable compilation for just the offending functions without touching anything else. So in my opinion shipping the compiler to prod should be pretty safe.

If you have the ability to, running an A/B test can help get more perf data and changes to engagement, as well as be a proxy to check if anything might be broken (ie some metric goes down after shipping the compiler to prod)

@AlessioGr
Copy link
Member Author

For context we've been using the compiler in prod on many Meta apps for several months now without any major issues. The issues we have seen usually involve some sketchy React pattern (that the compiler can't always detect statically) that results in unexpected behavior. However in those cases once the offending component/hook(s) are found it's trivial to disable compilation for just the offending functions without touching anything else. So in my opinion shipping the compiler to prod should be pretty safe.

Awesome, that's reassuring enough, thank you!

If you have the ability to, running an A/B test can help get more perf data and changes to engagement, as well as be a proxy to check if anything might be broken (ie some metric goes down after shipping the compiler to prod)

We're planning to do performance testing in the future. Will definitely include tests with / without react compiler

@AlessioGr AlessioGr changed the title feat: run ui and lexical through react compiler feat: pre-compile ui and richtext-lexical with react compiler Aug 19, 2024
@AlessioGr AlessioGr merged commit ebd43c7 into beta Aug 19, 2024
51 checks passed
@AlessioGr AlessioGr deleted the feat/react-compile-pkgs branch August 19, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants