refactor: noop effect hooks on main thread#2424
Conversation
🦋 Changeset detectedLatest commit: ebe224d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a Changeset and a shared Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
68e40d1 to
c93abfe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react/runtime/src/hooks/react.ts`:
- Line 163: The ternary for useImperativeHandle is inverted: change the
expression so useImperativeHandle is a no-op on the main thread (when
__BACKGROUND__ is false) like useEffect/useLayoutEffect; specifically update the
assignment involving useImperativeHandle, usePreactUseImperativeHandle, noop and
__BACKGROUND__ so the noop branch runs when __BACKGROUND__ is false and the real
implementation (usePreactUseImperativeHandle) runs when __BACKGROUND__ is true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2efedd07-81ae-430c-9384-6d5e4dec4e96
📒 Files selected for processing (3)
.changeset/neat-pumas-thank.mdpackages/react/runtime/src/hooks/react.tspackages/react/runtime/src/utils.ts
Merging this PR will degrade performance by 14.74%
Performance Changes
Comparing Footnotes
|
React MTF Example#233 Bundle Size — 206.05KiB (-0.59%).ebe224d(current) vs c97d61a main#232(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch fix/main-thread-effect-hooks-noo... Project dashboard Generated by RelativeCI Documentation Report issue |
Web Explorer#8675 Bundle Size — 728.84KiB (0%).ebe224d(current) vs c97d61a main#8674(baseline) Bundle metrics
Bundle size by type
|
| Current #8675 |
Baseline #8674 |
|
|---|---|---|
384.62KiB |
384.62KiB |
|
342.07KiB |
342.07KiB |
|
2.16KiB |
2.16KiB |
Bundle analysis report Branch fix/main-thread-effect-hooks-noo... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#219 Bundle Size — 590.71KiB (-0.13%).ebe224d(current) vs c97d61a main#218(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch fix/main-thread-effect-hooks-noo... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#7100 Bundle Size — 236.85KiB (-0.35%).ebe224d(current) vs c97d61a main#7099(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch fix/main-thread-effect-hooks-noo... Project dashboard Generated by RelativeCI Documentation Report issue |
c93abfe to
f922cca
Compare
Summary by CodeRabbit
Checklist