feat: support path() for createQuerySelector
#1456
Conversation
🦋 Changeset detectedLatest commit: 1bac01d The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis change introduces a new API, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
PupilTong
requested review from
Sherry-hue and
Copilot
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the path() method in createQuerySelector by implementing a new getPathInfo API that retrieves the DOM path from a target node to the root.
- Implements
getPathInfoAPI across the web platform stack with cross-thread RPC communication - Adds path traversal logic that captures node information (tag, id, class, dataset, index) from target to root
- Includes comprehensive test coverage to verify the path extraction functionality
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| createGetPathInfo.ts | Creates background thread handler for getPathInfo RPC calls |
| createNativeApp.ts | Integrates getPathInfo into the NativeApp interface |
| index.jsx | Test component that validates path extraction from nested DOM structure |
| react.spec.ts | Adds test case for api-get-path-info functionality |
| startBackground.ts | Registers getPathInfo handler in background thread |
| registerGetPathInfoHandler.ts | UI thread handler that implements DOM path traversal logic |
| NativeApp.ts | Updates type definitions to include getPathInfo method and flexible data type |
| endpoints.ts | Defines RPC endpoint for getPathInfo communication |
| get-path-info-api.md | Documents the new feature in changeset |
packages/web-platform/web-core/src/uiThread/crossThreadHandlers/registerGetPathInfoHandler.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-tests/tests/react.spec.ts (1)
515-521: LGTM! Test follows established patterns.The new
api-get-path-infotest case correctly implements the standard testing pattern used throughout this file and properly validates the API functionality through visual indicators. The test aligns well with the PR objective of supportingpath()forcreateQuerySelector.Consider enhancing test coverage by adding:
- Error case testing (invalid selectors, non-existent elements)
- Verification of actual path information structure returned by the API
- Edge cases like deeply nested elements or complex DOM structures
The current implementation is sufficient for basic functionality validation, but additional test cases would improve robustness.
packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx (1)
30-33: Consider more robust comparison logic.The current comparison assumes all expected nodes have defined
idproperties, but some nodes in the path (like the root view and page) don't have ids. Consider using a more explicit comparison:if ( - !node || node.tag !== expect.tag || node.id !== expect.id + !node || node.tag !== expect.tag || + (expect.id !== undefined && node.id !== expect.id) || + (expect.id === undefined && node.id !== undefined) || node.index !== expect.index ) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.changeset/get-path-info-api.md(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-core/src/uiThread/crossThreadHandlers/registerGetPathInfoHandler.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startBackground.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(2 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createGetPathInfo.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
📚 Learning: in the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsxpackages/web-platform/web-tests/tests/react.spec.ts
📚 Learning: the component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork ...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Applied to files:
packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx
📚 Learning: in the lynx-stack codebase, css selectors in ssr hydration are generated by their own packages, ensu...
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Applied to files:
packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx
📚 Learning: in the lynx-stack ssr implementation, each createlynxview instance is used to render once and then d...
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Applied to files:
packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx
📚 Learning: in the lynx-family/lynx-stack repository, the webpack patch (patches/[email protected]) was crea...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
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/get-path-info-api.md
📚 Learning: in the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-v...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react.spec.ts
🧬 Code Graph Analysis (3)
packages/web-platform/web-core/src/uiThread/startBackground.ts (1)
packages/web-platform/web-core/src/uiThread/crossThreadHandlers/registerGetPathInfoHandler.ts (1)
registerGetPathInfoHandler(30-98)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createGetPathInfo.ts (1)
createGetPathInfo(12-35)
packages/web-platform/web-constants/src/endpoints.ts (2)
packages/web-platform/web-worker-rpc/src/RpcEndpoint.ts (1)
createRpcEndpoint(128-142)packages/web-platform/web-constants/src/types/NativeApp.ts (1)
InvokeCallbackRes(100-103)
🔇 Additional comments (17)
packages/web-platform/web-core/src/uiThread/startBackground.ts (2)
22-22: LGTM!The import statement follows the established pattern and is placed appropriately with other cross-thread handler imports.
53-56: LGTM!The handler registration follows the established pattern and correctly passes the required
backgroundRpcandshadowRootparameters.packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
31-31: LGTM!The import statement is correctly structured and placed appropriately with other cross-thread handler imports.
137-137: LGTM!The
getPathInfoproperty addition correctly integrates the new API using thecreateGetPathInfohelper with the appropriateuiThreadRpcparameter, following the established pattern in the codebase..changeset/get-path-info-api.md (1)
1-13: LGTM! Changeset properly documents the new feature.The changeset correctly identifies all affected packages and provides a concise summary of the implementation. The patch version increments are appropriate for this new feature addition.
packages/web-platform/web-constants/src/endpoints.ts (1)
127-136: LGTM! Endpoint definition follows established patterns.The
getPathInfoEndpointis correctly defined with appropriate parameter types for node identification and matches the signature pattern used by similar endpoints likeinvokeUIMethodEndpoint. The return typeInvokeCallbackResand RPC flags are appropriate for this async operation.packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createGetPathInfo.ts (1)
12-35: LGTM! Clean RPC wrapper implementation with proper error handling.The function correctly maps the API parameters to the RPC endpoint call and provides appropriate error handling by logging failures and returning structured error responses to the callback.
packages/web-platform/web-constants/src/types/NativeApp.ts (2)
102-102: Type change improves flexibility for structured data.Changing
datafromstring | undefinedtounknown | undefinedmakes sense for the path info API which returns structured path data rather than simple strings. This change also makes the type more flexible for future APIs.
153-160: LGTM! Method signature is consistent with existing APIs.The
getPathInfomethod follows the established pattern used by other NativeApp methods likeinvokeUIMethod, with appropriate parameter types for node identification and a callback-based async interface.packages/web-platform/web-tests/tests/react/api-get-path-info/index.jsx (1)
7-67: Well-structured test with comprehensive path validation.The test component effectively validates the
getPathInfoAPI by:
- Creating a specific DOM hierarchy that matches the expected path structure
- Using the new
lynx.createSelectorQuery().select('#target').path()API- Comparing the returned path against expected node properties (tag, id, index)
- Providing clear visual feedback for test results
The expected path structure correctly represents the DOM hierarchy from the target text node up to the root.
packages/web-platform/web-core/src/uiThread/crossThreadHandlers/registerGetPathInfoHandler.ts (7)
1-13: LGTM!The imports are well-organized and all appear to be used in the implementation. The licensing header and import structure follow the project's conventions.
15-28: LGTM!The type definitions are well-structured and clearly documented. The optional fields are appropriately marked, and the PathInfo documentation clearly explains the path ordering from target to root.
30-45: LGTM!The function signature and initial setup are well-structured. The error code defaults to
UNKNOWNappropriately, and the parameter destructuring matches the expected RPC endpoint signature.
46-53: LGTM!The queryNodes call correctly passes through all parameters and provides appropriate success and error callbacks. This follows the expected pattern for the utility function.
54-83: Path construction algorithm looks solid.The algorithm correctly walks up the DOM tree from target to root, extracting relevant node information at each step. The try/catch wrapper provides good error protection, and the break conditions appropriately handle both semantic ('page' tag) and structural (shadowRoot) boundaries.
84-96: LGTM!The error handling is comprehensive with proper logging context, and the return structure correctly implements the
InvokeCallbackResinterface. All code paths appropriately set error codes and return consistent response formats.
61-61: Add null check for lynxTagAttribute.The non-null assertion (
!) assumes thatlynxTagAttributeis always present. Consider adding a null check to handle cases where the attribute might be missing.- const tag = currentNode.getAttribute(lynxTagAttribute)!; + const tag = currentNode.getAttribute(lynxTagAttribute); + if (!tag) { + throw new Error(`Missing ${lynxTagAttribute} on element`); + }⛔ Skipped due to learnings
Learnt from: PupilTong PR: lynx-family/lynx-stack#1292 File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151 Timestamp: 2025-07-15T10:00:56.154Z Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
PupilTong
force-pushed
the
p/hw/nativeapp-getpaths
branch
from
cca78ac to
c97354c
Compare
React Example#3878 Bundle Size — 235.26KiB (0%).1bac01d(current) vs ead12c9 main#3868(baseline) Bundle metrics
|
| Current #3878 |
Baseline #3868 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
159 |
159 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.81% |
45.81% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #3878 |
Baseline #3868 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
89.5KiB |
89.5KiB |
Bundle analysis report Branch PupilTong:p/hw/nativeapp-getpath... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#3873 Bundle Size — 343.48KiB (+0.24%).1bac01d(current) vs ead12c9 main#3864(baseline) Bundle metrics
|
| Current #3873 |
Baseline #3864 |
|
|---|---|---|
 Initial JS |
142.95KiB(+0.42%) |
142.35KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
 Cache Invalidation |
50.4% |
0% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
| Modules |
212(+0.47%) |
211 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.97%(+0.25%) |
3.96% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #3873 |
Baseline #3864 |
|
|---|---|---|
| JS |
228.69KiB (+0.36%) |
227.87KiB |
| Other |
82.95KiB |
82.95KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch PupilTong:p/hw/nativeapp-getpath... Project dashboard
Generated by RelativeCI Documentation Report issue
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1456 will not alter performanceComparing Summary
Footnotes |
PupilTong
force-pushed
the
p/hw/nativeapp-getpaths
branch
2 times, most recently
from
f2ac120 to
396c7e7
Compare
PupilTong
force-pushed
the
p/hw/nativeapp-getpaths
branch
from
396c7e7 to
1bac01d
Compare
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 - Supports `recyclable` attribute in `<list-item>` to control whether the list item is recyclable. The `recyclable` attribute depends on Lynx Engine 3.4 or later. ([#1388](#1388)) ```jsx <list-item recyclable={false} /> ``` - feat: Support using a host element as direct child of Suspense ([#1455](#1455)) - Add profile in production build: ([#1336](#1336)) 1. `diff:__COMPONENT_NAME__`: how long ReactLynx diff took. 2. `render:__COMPONENT_NAME__`: how long your render function took. 3. `setState`: an instant trace event, indicate when your setState was called. NOTE: `__COMPONENT_NAME__` may be unreadable when minified, setting `displayName` may help. - Add `onBackgroundSnapshotInstanceUpdateId` event on dev for Preact Devtools to keep the correct snapshotInstanceId info. ([#1173](#1173)) - fix: Prevent error when spreading component props onto an element ([#1459](#1459)) - fix: Correctly check for the existence of background functions in MTS ([#1416](#1416)) ```ts function handleTap() { "main thread"; // The following check always returned false before this fix if (myHandleTap) { runOnBackground(myHandleTap)(); } } ``` ## @lynx-js/[email protected] ### Patch Changes - Remove the experimental `provider` option. ([#1432](#1432)) - Add `output.filename.wasm` and `output.filename.assets` options. ([#1449](#1449)) - fix deno compatibility ([#1412](#1412)) - Should call the `api.onCloseBuild` hook after the build finished. ([#1446](#1446)) - Bump Rsbuild v1.4.15. ([#1423](#1423)) - Support using function in `output.filename.*`. ([#1449](#1449)) ## [email protected] ### Patch Changes - Support ESLint for ReactLynx templates ([#1274](#1274)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`c8ce6aa`](c8ce6aa)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the `Package subpath './compat' is not defined by "exports"` error. ([#1460](#1460)) ## @lynx-js/[email protected] ### Patch Changes - Fix `GlobalEventEmitter` type definition, the `emit(eventName: string, data: unknown)` function should recevie an array typed `data` and pass as param list of listeners. ([#1479](#1479)) ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - fix: when `onNativeModulesCall` is delayed in mounting, the NativeModules execution result may be undefined. ([#1457](#1457)) - fix: `onNativeModulesCall` && `onNapiModulesCall` use getter to get. ([#1466](#1466)) - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. ## @lynx-js/[email protected] ### Patch Changes - feat: add autocomplete attribute support for x-input component ([#1444](#1444)) Implements autocomplete attribute forwarding from the x-input custom element to the internal HTML input element in the shadow DOM. This enables standard browser autocomplete functionality for x-input elements. - Add referrerpolicy attribute support to x-image web component ([#1420](#1420)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## [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>
2 participants
Summary by CodeRabbit
Checklist