fix: reduce redundant updates for handlers and gestures#2188
fix: reduce redundant updates for handlers and gestures#2188Yradex wants to merge 2 commits intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 23f46ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughAdds copy-on-commit preparation for worklets, spreads, and gestures; replaces in-place mutations with prepare-* helpers; ensures removed gestures clear native DOM state; introduces unit and snapshot tests and a changelog entry documenting reduced execId/snapshot churn for stable references. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 482ffeaefc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Yradex
force-pushed
the
wt/mts-support-use-callback-20260202
branch
from
482ffea to
77dec4b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/react/runtime/src/gesture/processGestureBagkround.ts`:
- Around line 45-51: The committedCallbacks cloning assumes every callback is
non-null and calls prepareWorkletForCommit(...)! which will throw when a
callback is null/undefined; update the loop over committedCallbacks (the object
derived from baseGesture.callbacks) to check each committedCallbacks[name] for
null/undefined and only call prepareWorkletForCommit when it is present,
otherwise leave the entry as-is (remove the non-null assertions and skip calling
prepareWorkletForCommit for null/undefined values) so null callbacks are
preserved and not spread into worklets.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Implement copy-on-commit for worklets, gestures, and spread props to avoid background-side mutation. - Prevent _execId churn for stable references, reducing redundant native patches. - Fix gesture removal cleanup when removed from spread props. - Add regression tests for execId churn and gesture cleanup. - Add changeset for @lynx-js/react-runtime.
Yradex
force-pushed
the
wt/mts-support-use-callback-20260202
branch
from
77dec4b to
b09eaa9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/react/runtime/__test__/snapshot/mtf-execid-churn.test.jsx`:
- Around line 142-199: The test reads lynx.getNativeApp().callLepusMethod too
soon after the background render, causing a race; after render(<Comp tick={1}
/>, __root) in the "rerender with no semantic changes" block, await the
scheduled patch (e.g., await Promise.resolve() or flush pending
timers/microtasks) before switching back to main thread and accessing
lynx.getNativeApp().callLepusMethod.mock.calls so that
getSnapshotPatchFromPatchUpdateCall sees the finalized call.
🧹 Nitpick comments (1)
packages/react/runtime/__test__/snapshot/mtf-execid-churn.test.jsx (1)
12-16: Make helper failures explicit instead of a TypeError.
Ifcallorcall[1].datais missing, the current code throws a vague error. Adding explicit assertions makes failures easier to diagnose.Suggested change
function getSnapshotPatchFromPatchUpdateCall(call) { - const obj = call[1]; - const parsed = JSON.parse(obj.data); + expect(call, 'expected a patch update call').toBeTruthy(); + const obj = call[1]; + expect(obj?.data, 'expected patch payload').toBeTypeOf('string'); + const parsed = JSON.parse(obj.data); return parsed.patchList?.[0]?.snapshotPatch; }
…assertions and awaiting schedule.
Web Explorer#7528 Bundle Size — 383.66KiB (0%).23f46ef(current) vs e2d349e main#7526(baseline) Bundle metrics
|
| Current #7528 |
Baseline #7526 |
|
|---|---|---|
 Initial JS |
154.82KiB |
154.82KiB |
| Initial CSS |
35.05KiB |
35.05KiB |
 Cache Invalidation |
0% |
40.35% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
238 |
238 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
2.99% |
2.99% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7528 |
Baseline #7526 |
|
|---|---|---|
| JS |
252.76KiB |
252.76KiB |
| Other |
95.85KiB |
95.85KiB |
| CSS |
35.05KiB |
35.05KiB |
Bundle analysis report Branch Yradex:wt/mts-support-use-callba... Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/react/runtime/__test__/snapshot/mtf-execid-churn.test.jsx`:
- Around line 26-34: The test overrides a global value but never restores it;
capture the original SystemInfo.lynxSdkVersion in the beforeEach (e.g., const
originalLynxSdkVersion = SystemInfo.lynxSdkVersion) before setting
SystemInfo.lynxSdkVersion = '999.999', and then restore it in afterEach (set
SystemInfo.lynxSdkVersion = originalLynxSdkVersion) alongside
vi.restoreAllMocks() and elementTree.clear() to prevent cross-test leakage.
- Around line 20-24: Tests mutate global state via injectUpdateMainThread
(assigning globalThis['rLynxChange']) and patch Preact's options.commit via
replaceCommitHook/hook(), so add an afterAll() that deletes
globalThis['rLynxChange'] and restores options.commit to its original value (use
the original reference captured by hook() or reset options.commit to
undefined/default) in the mtf-execid-churn.test.jsx file so test globals are
cleaned up in addition to vi.restoreAllMocks().
CodSpeed Performance ReportMerging this PR will degrade performance by 22.78%Comparing Summary
Performance Changes
Footnotes
|
2 participants
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Checklist