Skip to content

feat: remove stale gestures when diff#2297

Open
f0rdream wants to merge 1 commit intolynx-family:mainfrom
f0rdream:gesture_stale_removal
Open

feat: remove stale gestures when diff#2297
f0rdream wants to merge 1 commit intolynx-family:mainfrom
f0rdream:gesture_stale_removal

Conversation

@f0rdream
Copy link
Collaborator

@f0rdream f0rdream commented Mar 3, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added gesture detector removal functionality to support complete gesture lifecycle management.
  • Bug Fixes

    • Fixed stale gesture persistence by implementing proper cleanup operations when gestures are removed or updated.
  • Tests

    • Enhanced gesture test coverage with improved lifecycle verification and state assertions.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).
  • Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

@f0rdream f0rdream requested review from HuJean, Yradex and hzy as code owners March 3, 2026 14:00
@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: 05b9dc0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@lynx-js/testing-environment Patch
@lynx-js/react Patch

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

This PR introduces gesture detector removal functionality across the React framework ecosystem. It adds the __RemoveGestureDetector PAPI binding, refactors gesture processing logic to handle deduplication and removal of stale detectors, expands type declarations and public APIs, enhances test coverage, and enables instrumentation profiling.

Changes

Cohort / File(s) Summary
Changesets
.changeset/all-crabs-smell.md, .changeset/fair-horses-worry.md
New changeset entries documenting patch releases for "@lynx-js/testing-environment" and "@lynx-js/react" to support gesture detector removal and cleanup.
Type Definitions and API Surface
packages/react/runtime/types/types.d.ts, packages/testing-library/testing-environment/etc/testing-environment.api.md
Declare and document the new __RemoveGestureDetector(node, id) function in global typings and update public API report for testing-environment package.
Core Gesture Processing
packages/react/runtime/src/gesture/processGesture.ts, packages/react/runtime/src/alog/elementPAPICall.ts
Refactored gesture processing to deduplicate base gestures and remove stale detectors via new helpers; added __RemoveGestureDetector to PAPI instrumentation list.
Testing Infrastructure and Public API
packages/react/runtime/__test__/utils/nativeMethod.ts, packages/testing-library/testing-environment/src/lynx/ElementPAPI.ts
Introduced per-element gesture detector tracking with map-based storage; added __RemoveGestureDetector, __GetGestureDetector, and __GetGestureDetectorIds helper methods.
Test Coverage
packages/react/runtime/__test__/snapshot/gesture.test.jsx
Enhanced gesture lifecycle tests with spies for __SetGestureDetector and __RemoveGestureDetector to verify detector creation, update, and removal sequences.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

  • feat: add gesture-runtime #1984: Introduces the same __RemoveGestureDetector API and gesture detector removal/wiring logic across framework typings and implementations.

Suggested labels

framework:React

Suggested reviewers

  • HuJean
  • Yradex
  • hzy

Poem

🐰 A rabbit hops through gesture trees,
Detectors set with gentle ease,
But when they fade, no more they stay—
RemoveGestureDetector clears the way! ✨
Stale gestures gone, the code runs clean,
The finest gesture flow we've seen! 🎭

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat: remove stale gestures when diff' is only partially related to the changeset. While the PR does implement gesture removal logic, the title is incomplete and vague—'when diff' is truncated or unclear phrasing that doesn't fully convey the actual change. Clarify the title to complete the thought, such as 'feat: remove stale gesture detectors on re-render' or 'feat: clean up removed gesture detectors', to better describe the core change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/react/runtime/__test__/snapshot/gesture.test.jsx (1)

360-360: Consider extracting repeated text element lookup helper.

The traversal __root.__element_root.children[0].children[0] is repeated multiple times and is brittle to tree-shape changes; a local helper would improve maintainability.

♻️ Optional refactor
+const getTextElement = () => __root.__element_root.children[0].children[0];
...
-const textElement = __root.__element_root.children[0].children[0];
+const textElement = getTextElement();

Also applies to: 702-702, 848-848, 1119-1119

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/__test__/snapshot/gesture.test.jsx` at line 360, The
test repeats the brittle traversal
__root.__element_root.children[0].children[0]; extract a small helper (e.g.,
getTextElement or findTextElement) that returns that node so all occurrences
call that function instead of duplicating the path; update usages at the
locations referencing __root.__element_root.children[0].children[0] to call the
helper, improving readability and making future tree-shape changes easier to
handle.
🤖 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/runtime/__test__/snapshot/gesture.test.jsx`:
- Line 360: The test repeats the brittle traversal
__root.__element_root.children[0].children[0]; extract a small helper (e.g.,
getTextElement or findTextElement) that returns that node so all occurrences
call that function instead of duplicating the path; update usages at the
locations referencing __root.__element_root.children[0].children[0] to call the
helper, improving readability and making future tree-shape changes easier to
handle.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6cbe5d8 and 05b9dc0.

📒 Files selected for processing (9)
  • .changeset/all-crabs-smell.md
  • .changeset/fair-horses-worry.md
  • packages/react/runtime/__test__/snapshot/gesture.test.jsx
  • packages/react/runtime/__test__/utils/nativeMethod.ts
  • packages/react/runtime/src/alog/elementPAPICall.ts
  • packages/react/runtime/src/gesture/processGesture.ts
  • packages/react/runtime/types/types.d.ts
  • packages/testing-library/testing-environment/etc/testing-environment.api.md
  • packages/testing-library/testing-environment/src/lynx/ElementPAPI.ts

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.

1 participant