fix(react/testing-library): align fireEvent with Lynx bubbling semantics#2532
fix(react/testing-library): align fireEvent with Lynx bubbling semantics#2532
Conversation
- TouchEvent-family events (tap, longtap, touchstart, touchmove, touchend, touchcancel, longpress) now default to bubbles: true, matching the Lynx runtime where these events propagate through capture/bubble phases. - Skip read-only Event accessors (bubbles/cancelable/composed) when applying EventInit via Object.assign — those are getters on Event.prototype and reassigning them throws TypeError in strict mode. - Add Event handler property semantics tests covering bind/catch/ capture-bind/capture-catch propagation and bubble defaults across the TouchEvent family.
🦋 Changeset detectedLatest commit: 66f0503 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTouch-event fireEvent helpers now default to bubbles: true; event construction omits Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/testing-library/src/__tests__/events.test.jsx (1)
314-330: Addlongtapto the default-bubbling ancestor test matrix.Line 314 currently validates touchstart/touchmove/touchend/touchcancel/longpress, but
longtapalso changed to defaultbubbles: trueinfire-event.ts. Including it here would close that regression gap.🧪 Proposed test update
- it.each(['touchstart', 'touchmove', 'touchend', 'touchcancel', 'longpress'])( + it.each(['longtap', 'touchstart', 'touchmove', 'touchend', 'touchcancel', 'longpress'])( '%s: bubbles to ancestor handlers by default', (eventName) => { const parent = vi.fn(); const childRef = createRef();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/testing-library/src/__tests__/events.test.jsx` around lines 314 - 330, Update the test matrix in the it.each call so the ancestor-bubbling test covers the 'longtap' event as well: inside the array passed to it.each (currently ['touchstart','touchmove','touchend','touchcancel','longpress']), add 'longtap' so that the loop invokes fireEvent[eventName](childRef.current) for 'longtap' and asserts parent handler was called; this ensures the change in fire-event.ts (default bubbles: true for longtap) is covered by the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/testing-library/src/__tests__/events.test.jsx`:
- Around line 314-330: Update the test matrix in the it.each call so the
ancestor-bubbling test covers the 'longtap' event as well: inside the array
passed to it.each (currently
['touchstart','touchmove','touchend','touchcancel','longpress']), add 'longtap'
so that the loop invokes fireEvent[eventName](childRef.current) for 'longtap'
and asserts parent handler was called; this ensures the change in fire-event.ts
(default bubbles: true for longtap) is covered by the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e101f9b8-2193-46b8-b249-8fe0d90e315e
📒 Files selected for processing (3)
.changeset/fix-testing-library-tap-bubbles.mdpackages/react/testing-library/src/__tests__/events.test.jsxpackages/react/testing-library/src/fire-event.ts
Merging this PR will not alter performance
Comparing Footnotes
|
Web Explorer#9256 Bundle Size — 900.02KiB (0%).66f0503(current) vs 3920afa main#9255(baseline) Bundle metrics
Bundle size by type
|
| Current #9256 |
Baseline #9255 |
|
|---|---|---|
495.88KiB |
495.88KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#814 Bundle Size — 196.54KiB (0%).66f0503(current) vs e15852f main#811(baseline) Bundle metrics
|
| Current #814 |
Baseline #811 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44.08% |
44.08% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #814 |
Baseline #811 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.31KiB |
85.31KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7682 Bundle Size — 225.38KiB (0%).66f0503(current) vs e15852f main#7679(baseline) Bundle metrics
|
| Current #7682 |
Baseline #7679 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.57% |
44.57% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7682 |
Baseline #7679 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.63KiB |
79.63KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React External#799 Bundle Size — 680.27KiB (0%).66f0503(current) vs 3920afa main#798(baseline) Bundle metrics
|
| Current #799 |
Baseline #798 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
Summary
tap,longtap,touchstart,touchmove,touchend,touchcancel,longpress) now default tobubbles: true, matching the Lynx runtime where every event whose handler signature isEventHandler<BaseTouchEvent<T>>in@lynx-js/typespropagates through capture/bubble phases. Other entries (bgload,transitionend, mouse/key/focus/blur/layout, …) keep their non-bubbling default — Lynx has no symmetric capture-bind/capture-catch API for them.Event.prototypeaccessors (bubbles,cancelable,composed) when applyingEventInitviaObject.assignafter construction — reassigning those getters throwsTypeErrorin strict mode. They're still applied via theEventInitdict.bind/catch/capture-bind/capture-catchhandler semantics and the TouchEvent-family bubble defaults inevents.test.jsx.Test plan
pnpm vitest run src/__tests__/events.test.jsx— 41/41 pass (was previously failing in strict mode on theObject.assign(event, init)call wheninitcarriedbubbles/cancelable/composed)Summary by CodeRabbit
Bug Fixes
Tests
Documentation