Skip to content

Conversation

@Sherry-hue
Copy link
Collaborator

@Sherry-hue Sherry-hue commented Nov 3, 2025

Summary by CodeRabbit

  • Bug Fixes

    • bindtap events now include accurate x and y coordinates in event detail, so tap handlers can access pointer position.
  • Tests

    • Added a test that verifies bindtap detail includes coordinates and that handlers respond accordingly.
  • Chores

    • Recorded a changeset entry and ensured offscreen event handling includes x and y in transferred event data.

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).

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025

🦋 Changeset detected

Latest commit: 43b634d

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

This PR includes changesets to release 11 packages
Name Type
@lynx-js/web-mainthread-apis Patch
@lynx-js/web-core-server Patch
@lynx-js/web-core Patch
@lynx-js/web-worker-runtime Patch
upgrade-rspeedy Patch
@lynx-js/web-rsbuild-server-middleware Patch
@lynx-js/web-constants Patch
@lynx-js/web-worker-rpc Patch
@lynx-js/web-style-transformer Patch
@lynx-js/rspeedy Patch
create-rspeedy 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 Nov 3, 2025

📝 Walkthrough

Walkthrough

This PR ensures bindtap/click events include mouse coordinates by extracting x and y into event.detail, adds a testsuite and test component verifying the behavior, updates offscreen document property collection to include x and y, and adds a changeset entry recording the patch-level fix.

Changes

Cohort / File(s) Summary
Core event fix
packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
Introduces a local detail variable and a type === 'click' branch that populates detail.x and detail.y from the MouseEvent, and returns the cross-thread event using this detail.
Offscreen event property list
packages/web-platform/offscreen-document/src/main/initOffscreenDocument.ts
Adds x and y to the set of collected event-related property names so coordinates are included during offscreen document initialization/transfer.
Tests & test app
packages/web-platform/web-tests/tests/react.spec.ts, packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx
Adds basic-bindtap-detail test and a test component that toggles color on tap only when e.detail.x and e.detail.y are numeric; test clicks the target twice and asserts color changes.
Changeset
.changeset/modern-windows-doubt.md
Adds a patch-level changeset documenting the fix to include x and y in bindtap event detail.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Mixed changes: small runtime logic, offscreen serialization list, and new tests.
  • Review focus:
    • Correctness of coordinate extraction and types in createCrossThreadEvent.ts.
    • Consistency of x/y presence across mainthread ↔ offscreen serialization.
    • Test robustness (timing, event synthesis) in basic-bindtap-detail.

Possibly related PRs

Suggested reviewers

  • PupilTong
  • gaoachao

Poem

"🐰 I hopped across threads, a tiny fix to try,
I tucked x and y beneath the click's shy sigh,
Pink then green the test hops through the day,
Cross-thread carrots pave the eventful way! ✨"

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix(web): bindtap detail should contain x, y' directly and clearly describes the main change in the changeset. It specifies that this is a fix to the bindtap functionality to ensure the detail object includes x and y coordinates. This aligns perfectly with the core changes across all modified files: updates to createCrossThreadEvent.ts to populate detail with mouse coordinates, addition of initOffscreenDocument.ts to include x and y properties, test coverage via a new test case and test component, and a corresponding changeset entry.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22dc385 and 43b634d.

📒 Files selected for processing (5)
  • .changeset/modern-windows-doubt.md (1 hunks)
  • packages/web-platform/offscreen-document/src/main/initOffscreenDocument.ts (1 hunks)
  • packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts (3 hunks)
  • packages/web-platform/web-tests/tests/react.spec.ts (1 hunks)
  • packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/web-platform/web-mainthread-apis/src/utils/createCrossThreadEvent.ts
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md

📄 CodeRabbit inference engine (AGENTS.md)

For contributions, generate and commit a Changeset describing your changes

Files:

  • .changeset/modern-windows-doubt.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
📚 Learning: 2025-10-11T06:16:12.517Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.

Applied to files:

  • packages/web-platform/web-tests/tests/react.spec.ts
  • .changeset/modern-windows-doubt.md
  • packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.

Applied to files:

  • packages/web-platform/web-tests/tests/react.spec.ts
  • packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.

Applied to files:

  • .changeset/modern-windows-doubt.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.

Applied to files:

  • .changeset/modern-windows-doubt.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.

Applied to files:

  • .changeset/modern-windows-doubt.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.

Applied to files:

  • .changeset/modern-windows-doubt.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.

Applied to files:

  • .changeset/modern-windows-doubt.md
🧬 Code graph analysis (2)
packages/web-platform/web-tests/tests/react.spec.ts (1)
packages/web-platform/web-tests/tests/coverage-fixture.ts (1)
  • test (11-60)
packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx (1)
packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx (1)
  • App (6-27)
🔇 Additional comments (4)
.changeset/modern-windows-doubt.md (1)

1-5: LGTM! Changeset correctly documents the fix.

The changeset follows the correct format and appropriately describes the patch-level fix for ensuring bindtap event detail includes x and y coordinates.

packages/web-platform/offscreen-document/src/main/initOffscreenDocument.ts (1)

21-22: LGTM! Correctly adds x and y to event property tracking.

Adding x and y to otherPropertyNames ensures these MouseEvent coordinates are included in the properties transferred to the offscreen context, enabling the bindtap event detail to contain mouse position information.

packages/web-platform/web-tests/tests/react.spec.ts (1)

121-131: LGTM! Test correctly validates bindtap detail includes coordinates.

The test case mirrors the structure of basic-bindtap but validates different behavior—ensuring event.detail contains numeric x and y coordinates. The test component only toggles color when these properties are present and numeric, so if the fix is incomplete, the assertions will fail.

packages/web-platform/web-tests/tests/react/basic-bindtap-detail/index.jsx (1)

7-11: LGTM! Validation logic correctly verifies detail coordinates.

The handleTap function properly validates that both e.detail.x and e.detail.y exist and are numeric before toggling the color. This ensures the test will fail if the bindtap event detail doesn't include valid coordinate properties, providing strong verification of the fix.


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.

PupilTong
PupilTong previously approved these changes Nov 3, 2025
@Sherry-hue Sherry-hue enabled auto-merge (squash) November 3, 2025 09:55
@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #1913 will improve performances by 6.62%

Comparing Sherry-hue:fix/bindtap-detail (43b634d) with main (c282f54)

Summary

⚡ 1 improvement
✅ 61 untouched
⏩ 3 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
basic-performance-small-css 7.4 ms 6.9 ms +6.62%

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@relativeci
Copy link

relativeci bot commented Nov 3, 2025

Web Explorer

#6086 Bundle Size — 366.7KiB (+0.01%).

43b634d(current) vs c282f54 main#6080(baseline)

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
#6086
     Baseline
#6080
Regression  Initial JS 146.2KiB(~+0.01%) 146.19KiB
No change  Initial CSS 32.22KiB 32.22KiB
Change  Cache Invalidation 46.79% 48.46%
No change  Chunks 8 8
No change  Assets 8 8
Change  Modules 219(-0.45%) 220
No change  Duplicate Modules 16 16
No change  Duplicate Code 3.21% 3.21%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#6086
     Baseline
#6080
Regression  JS 240.68KiB (+0.02%) 240.64KiB
No change  Other 93.8KiB 93.8KiB
No change  CSS 32.22KiB 32.22KiB

Bundle analysis reportBranch Sherry-hue:fix/bindtap-detailProject dashboard


Generated by RelativeCIDocumentationReport issue

@relativeci
Copy link

relativeci bot commented Nov 3, 2025

React Example

#6090 Bundle Size — 237.45KiB (0%).

43b634d(current) vs c282f54 main#6084(baseline)

Bundle metrics  no changes
                 Current
#6090
     Baseline
#6084
No change  Initial JS 0B 0B
No change  Initial CSS 0B 0B
No change  Cache Invalidation 0% 0%
No change  Chunks 0 0
No change  Assets 4 4
No change  Modules 165 165
No change  Duplicate Modules 67 67
No change  Duplicate Code 46.77% 46.77%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#6090
     Baseline
#6084
No change  IMG 145.76KiB 145.76KiB
No change  Other 91.69KiB 91.69KiB

Bundle analysis reportBranch Sherry-hue:fix/bindtap-detailProject dashboard


Generated by RelativeCIDocumentationReport issue

@Sherry-hue Sherry-hue merged commit fece7d0 into lynx-family:main Nov 3, 2025
44 checks passed
colinaaa pushed a commit that referenced this pull request Nov 9, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @lynx-js/[email protected]

### Patch Changes

- During hydration, replace update with insert + remove for same-type
`<list-item />` with different `item-key` so the Lynx Engine detects
changes. ([#1598](#1598))

    ```html
    Hydrate List B into List A: List A:
    <list>
      <list-item item-key="a">hello</list-item>
      <list-item item-key="a">world</list-item>
    </list>

    List B:
    <list>
      <list-item item-key="a1">hello</list-item>
      <list-item item-key="a2">world</list-item>
    </list>
    ```

Previously this case was hydrated as an update; it is now emitted as
insert + remove to ensure SDK detection.

- Bump `swc_core` v47.
([#1916](#1916))

- Pass sourcemap generated by rspack to swc transformer.
([#1910](#1910))

- When engineVersion is greater than or equal to 3.1, use
`__SetAttribute` to set text attribute for text node instead of creating
a raw text node.
([#1880](#1880))

- Add profile for list `update-list-info`.
([#1480](#1480))

- Support testing React Compiler in testing library. Enable React
Compiler by setting the `experimental_enableReactCompiler` option of
`createVitestConfig` to `true`.
([#1269](#1269))

    ```js
    import { defineConfig, mergeConfig } from "vitest/config";
import { createVitestConfig } from
"@lynx-js/react/testing-library/vitest-config";

    const defaultConfig = await createVitestConfig({
      runtimePkgName: "@lynx-js/react",
      experimental_enableReactCompiler: true,
    });

    export default mergeConfig(defaultConfig, config);
    ```

## @lynx-js/[email protected]

### Patch Changes

-   Updated dependencies \[]:
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- fix: print out the output chunk urls
([#1921](#1921))

## @lynx-js/[email protected]

### Patch Changes

- When engineVersion is greater than or equal to 3.1, use
`__SetAttribute` to set text attribute for text node instead of creating
a raw text node.
([#1880](#1880))

- Add `react-compiler-runtime` to `resolve.dedupe`.
([#1269](#1269))

With this change you can setup [React
Compiler](https://react.dev/learn/react-compiler) for ReactLynx by
`pluginBabel`:

    ```js
    import { defineConfig } from "@lynx-js/rspeedy";
    import { pluginBabel } from "@rsbuild/plugin-babel";

    export default defineConfig({
      plugins: [
        pluginBabel({
          include: /\.(?:jsx|tsx)$/,
          babelLoaderOptions(opts) {
            opts.plugins?.unshift([
              "babel-plugin-react-compiler",
// See https://react.dev/reference/react-compiler/configuration for
config
              {
                // ReactLynx only supports target to version 17
                target: "17",
              },
            ]);
          },
        }),
      ],
    });
    ```

- Updated dependencies
\[[`e7d186a`](e7d186a),
[`0d7a4c3`](0d7a4c3)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- feat: add \_\_GetSourceMapRelease API for nativeApp.
([#1923](#1923))

-   Updated dependencies \[]:
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- Updated dependencies
\[[`fece7d0`](fece7d0),
[`e1db63f`](e1db63f),
[`ebc1a60`](ebc1a60)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- fix: define x-foldview-slot-drag-ng typo.
([#1915](#1915))

- feat: 1. Added support for the list `estimated-main-axis-size-px`
property; the width and height of `list-item` are no longer required.
([#1911](#1911))

2. Fixed an issue where the list `lower-threshold-item-count` event
would not trigger when using a horizontal layout under a waterfall
layout.

3. Fixed an issue where calling the list `autoScroll` method in
`useEffect` might not scroll.

4. Fixed an issue where the `scrolltolower` event might not be triggered
in waterfall, because the lower styles was not updated in
`registerEventEnableStatusChangeHandler`.

-   Updated dependencies \[]:
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- fix: define x-foldview-slot-drag-ng typo.
([#1915](#1915))

## @lynx-js/[email protected]

### Patch Changes

- feat: update @lynx-js/web-elements to 0.8.10
([#1914](#1914))

## @lynx-js/[email protected]

### Patch Changes

- fix: The `e.detail` in the `bindtap` callback needs to correctly
include `x` and `y`.
([#1913](#1913))

- Updated dependencies
\[[`ebc1a60`](ebc1a60)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- fix: `this` may be undefined in Card().
([#1922](#1922))

- feat: add \_\_GetSourceMapRelease API for nativeApp.
([#1923](#1923))

- Updated dependencies
\[[`fece7d0`](fece7d0),
[`ebc1a60`](ebc1a60)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- Pass sourcemap generated by rspack to swc transformer.
([#1910](#1910))

- When engineVersion is greater than or equal to 3.1, use
`__SetAttribute` to set text attribute for text node instead of creating
a raw text node.
([#1880](#1880))

## [email protected]



## @lynx-js/[email protected]



## [email protected]



## @lynx-js/[email protected]



## @lynx-js/[email protected]



## @lynx-js/[email protected]



## @lynx-js/[email protected]



## @lynx-js/[email protected]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants