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

chore: setup react compiler test and linting #6730

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

stipsan
Copy link
Member

@stipsan stipsan commented May 21, 2024

Description

Adds support for the React Compiler in the test-next-studio. Also adds the eslint-plugin-react-compiler plugin, which warns us about areas in the codebase that the compiler will skip due to either breaking the rule of hooks, or the code being structured in a way where deps can't be inferred.

Components that are memoized by the Compiler gets their own little sparkly badge in the React Devtools extension:
image

Why add the Compiler? Even though we can't fully reap the benefits of it until we're ready to drop react v18 and make v19 the new baseline, it's useful to be able to track how much perf is improved by the Compiler.
It's also usually a good thing for react v18 perf to refactor the codebase to fix hooks bugs, and to make the code structured so the compiler can fully understand it and optimize it.

We won't see a huge difference right away, as our codebase are in a state that's the combination of:
a) components that would benefit the most from the compiler is currently skipped by it (see pnpm check:react-compiler and errors from react-compiler/react-compiler to see which ones, for example )
b) most of the components that can be optimized by react-compiler and see a boost are already optimized by hand with useMemo, useCallback and memo.

We can work on a) over time and it benefits react v18 users as well as v19. While b) is something we can reap once we make v19 the baseline and we can delete all handwritten useMemo, useCallback and memo instances 🙌

What to review

The change in useSyncValue should be fine, as it's functionally equivalent it's only refactored so it can be understood by the compiler.

Testing

Existing tests should suffice, it only affects the dev/test-next-studio project. There's also a new CI check that tells us if the compiler can run on the full codebase or not.

Notes for release

N/A

Copy link

vercel bot commented May 21, 2024

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

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 2:04pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 2:04pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 2:04pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 2:04pm

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented May 21, 2024

Component Testing Report Updated May 22, 2024 2:06 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 35s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 7s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 31s 11 7 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 8s 3 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 37s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 15s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 4s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 7s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 21s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 15s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 31s 12 0 0

@stipsan stipsan force-pushed the test-react-compiler branch 2 times, most recently from a7eccce to 48960a5 Compare May 21, 2024 20:51
@stipsan stipsan marked this pull request as ready for review May 21, 2024 20:51
@stipsan stipsan requested a review from a team May 21, 2024 20:51
@stipsan stipsan requested a review from a team as a code owner May 21, 2024 20:51
@stipsan stipsan requested review from jordanl17 and binoy14 and removed request for a team May 21, 2024 20:51
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/babel-plugin-react-compiler@0.0.0-experimental-592953e-20240517 None +1 5.55 MB sugarpirate

View full report↗︎

binoy14
binoy14 previously approved these changes May 22, 2024
Copy link
Contributor

@binoy14 binoy14 left a comment

Choose a reason for hiding this comment

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

Changes makes sense to me, probably need a rebase?

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