refactor: change code structure for improved readability and maintainibility#2004
Conversation
🦋 Changeset detectedLatest commit: 4059aa5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
📝 WalkthroughWalkthroughRefactors many class-private members from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Areas to pay extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (1)
31-71: Visibility change is justified fornoUnusedLocalscompatibility; reconsider defensive error handling per team patterns.The underscore prefix for decorated attribute handlers (
_handleSrc,_handleLoop,_handlePauseOnHide) is the correct approach—decorated methods fail withnoUnusedLocalswhen private, so this aligns with the decorator registration pattern established in the file.However, the try-catch wrapper around
JSON.parseintroduces defensive runtime error handling that conflicts with the team's established preference for compile-time type safety without runtime exception wrapping (per similar patterns in web-elements attribute handlers). Consider whether input validation should occur at a higher layer, or if the error should propagate rather than silently fallback to an empty object.packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
59-60: Inconsistent naming convention for new field.Line 59 introduces
#setTypeusing the private#syntax, while the entire PR systematically refactors from#to_prefix. This creates an inconsistency—either this field should follow the new_setTypeconvention, or there should be a documented reason for the exception.Note: Line 60's
#setInputmodeis pre-existing and may also warrant updating for consistency.🔎 Apply this diff to maintain naming consistency:
- #setType = bindToAttribute(this.#getInputElement, 'type'); - #setInputmode = bindToAttribute(this.#getInputElement, 'inputmode'); + _setType = bindToAttribute(this.#getInputElement, 'type'); + _setInputmode = bindToAttribute(this.#getInputElement, 'inputmode'); @registerAttributeHandler('type', true) _handleType(value: string | null) { // ... type handling logic ... - this.#setInputmode(inputMode); - this.#setType(inputType); + this._setInputmode(inputMode); + this._setType(inputType); }packages/web-platform/web-elements/src/XList/XListEvents.ts (1)
355-389: Critical: Undefined variablescrollon line 366.Line 358 was modified to exclude
lynxscrollfrom the destructuring, but line 366 references an undefined variablescroll. This will cause aReferenceErrorat runtime when the scroll event logic is triggered.🔎 Apply this fix to line 366:
- if (scroll !== null || this.#enableScrollEnd) { + if (this.#eventSwitches.lynxscroll || this.#enableScrollEnd) {
♻️ Duplicate comments (1)
packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
68-93: Same visibility change pattern as above.This method follows the same private-to-public renaming pattern as
_handleComplexEventEnableAttributes. The same verification and encapsulation considerations apply.
🧹 Nitpick comments (3)
packages/web-platform/web-elements/src/XList/ListItemAttributes.ts (1)
23-27: Verify the intentional weakening of encapsulation.The field has been renamed from
#handlerEstimatedMainAxisSizePx(truly private using#syntax) to_handlerEstimatedMainAxisSizePx(public field with underscore convention). This expands the API surface and weakens encapsulation.While the underscore prefix signals "internal use" by convention, it provides no enforcement—the field is now publicly accessible. If this systematic change (observed across multiple files per the summary) is required for build tool compatibility with
@rsbuild/plugin-source-buildor decorator behavior, please document the reasoning in the PR description or add a code comment explaining why true private fields cannot be used.If true privacy is not required, consider adding a JSDoc comment:
@registerAttributeHandler('estimated-main-axis-size-px', true) + /** + * @internal - Used by decorator system, do not access directly + */ _handlerEstimatedMainAxisSizePx = bindToStyle( () => this.#dom, '--estimated-main-axis-size-px', (v) => `${parseFloat(v)}px`, );packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts (1)
47-52: Pre-existing logic issue: incorrect orientation check for horizontal scrolling.Line 51 checks
scrollOrientation === 'vertical'forleftScrollDistance, but horizontal scrolling should check forscrollOrientation === 'horizontal'instead. This could cause incorrect behavior when the scroll orientation is explicitly set to 'vertical'.🔎 Apply this diff to fix the orientation check:
const leftScrollDistance = (attributeName === 'scroll-left' || attributeName === 'initial-scroll-offset') && (scrollX === '' || scrollX === 'true' - || scrollOrientation === 'vertical' + || scrollOrientation === 'horizontal' || scrollOrientation === 'both');packages/web-platform/web-elements-compat/package.json (1)
18-21: Confirm whetherlinear-compat.cssshould be part of the public API.The
./LinearContainer/*wildcard currently exposes bothLinearContainer.tsandlinear-compat.cssas public imports. Verify this is intentional—especially whether the CSS file should be directly importable by consumers via@lynx-js/web-elements-compat/LinearContainer/linear-compat.css.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-compat/package.json(1 hunks)packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/elements.css(1 hunks)packages/web-platform/web-elements/index.css(1 hunks)packages/web-platform/web-elements/package.json(1 hunks)packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts(4 hunks)packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts(3 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts(0 hunks)packages/web-platform/web-elements/src/XImage/DropShadow.ts(1 hunks)packages/web-platform/web-elements/src/XImage/ImageEvents.ts(2 hunks)packages/web-platform/web-elements/src/XImage/ImageSrc.ts(2 hunks)packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts(5 hunks)packages/web-platform/web-elements/src/XInput/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XInput/XInputAttribute.ts(2 hunks)packages/web-platform/web-elements/src/XInput/XInputEvents.ts(3 hunks)packages/web-platform/web-elements/src/XList/ListItemAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(7 hunks)packages/web-platform/web-elements/src/XList/XListWaterfall.ts(2 hunks)packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts(2 hunks)packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts(2 hunks)packages/web-platform/web-elements/src/XSvg/XSvg.ts(3 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts(1 hunks)packages/web-platform/web-elements/src/XText/InlineImage.ts(1 hunks)packages/web-platform/web-elements/src/XText/RawText.ts(1 hunks)packages/web-platform/web-elements/src/XText/XTextTruncation.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts(3 hunks)packages/web-platform/web-elements/src/XView/BlurRadius.ts(1 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-elements/tsconfig.json(0 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts
- packages/web-platform/web-elements/tsconfig.json
🧰 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/heavy-birds-beg.md
🧠 Learnings (18)
📓 Common learnings
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.
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.
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/index.csspackages/web-platform/web-elements/elements.csspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.tspackages/web-platform/web-elements/src/XSvg/XSvg.tspackages/web-platform/web-elements/package.json
📚 Learning: 2025-07-16T06:28:26.463Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 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-elements/index.csspackages/web-platform/web-elements/elements.css
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/web-platform/web-elements/index.css.changeset/heavy-birds-beg.md
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-worker-rpc/package.jsonpackages/web-platform/web-elements-reactive/package.jsonpackages/web-platform/web-elements/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-worker-rpc/package.jsonpackages/web-platform/web-elements-reactive/package.jsonpackages/web-platform/web-elements/package.json
📚 Learning: 2025-08-18T08:46:20.001Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1547
File: packages/rspeedy/core/src/config/loadConfig.ts:11-11
Timestamp: 2025-08-18T08:46:20.001Z
Learning: `#register` and similar imports starting with "#" are Node.js subpath imports defined in the "imports" field of package.json, not TypeScript path mapping aliases. These are natively supported by both Node.js and TypeScript without requiring additional tsconfig.json configuration like "moduleResolution" or "resolvePackageJsonImports" settings.
Applied to files:
packages/web-platform/web-worker-rpc/package.json
📚 Learning: 2025-07-16T06:25:41.055Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts:214-217
Timestamp: 2025-07-16T06:25:41.055Z
Learning: In the lynx-stack codebase, CSS strings produced by `genCssContent` are considered trusted developer input, so additional sanitization/escaping is not required.
Applied to files:
packages/web-platform/web-elements/elements.css
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XInput/XInputAttribute.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.tspackages/web-platform/web-elements/src/XText/InlineImage.tspackages/web-platform/web-elements/src/XList/XListWaterfall.tspackages/web-platform/web-elements/src/XText/RawText.tspackages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.tspackages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.tspackages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.tspackages/web-platform/web-elements/src/ScrollView/ScrollAttributes.tspackages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XText/XTextTruncation.tspackages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.tspackages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.tspackages/web-platform/web-elements/src/XList/XListEvents.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.tspackages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts
📚 Learning: 2025-11-11T08:05:14.163Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.163Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-elements/src/XInput/XInputAttribute.tspackages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.tspackages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.tspackages/web-platform/web-elements/src/XList/XListEvents.ts
📚 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:
packages/web-platform/web-elements-reactive/package.json.changeset/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 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/heavy-birds-beg.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/heavy-birds-beg.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/heavy-birds-beg.md
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
.changeset/heavy-birds-beg.md
📚 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/heavy-birds-beg.md
📚 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-elements/src/XInput/XInputEvents.ts
🧬 Code graph analysis (10)
packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindSwitchToEventListener(5-5)packages/web-platform/web-elements-reactive/src/bindSwitchToEventListener.ts (1)
bindSwitchToEventListener(8-32)
packages/web-platform/web-elements/src/XImage/ImageSrc.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (1)
registerEventEnableStatusChangeHandler(24-24)packages/web-platform/web-elements-reactive/src/registerEventStatusChangedHandler.ts (1)
registerEventEnableStatusChangeHandler(16-24)
packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToStyle(8-8)registerAttributeHandler(20-20)packages/web-platform/web-elements-reactive/src/registerAttributeHandler.ts (1)
registerAttributeHandler(19-30)
packages/web-platform/web-elements/src/XImage/DropShadow.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (47)
packages/web-platform/web-elements/src/XView/BlurRadius.ts (1)
23-24: LGTM! Decorator-compatible visibility for noUnusedLocals.The change from
#handleBlurRadiusto_handleBlurRadiusis appropriate. TypeScript's private field syntax (#) makes members inaccessible to decorators, and withnoUnusedLocalsenabled, the compiler might flag decorator-only methods as unused. Using the underscore convention maintains the private-by-convention semantics while allowing@registerAttributeHandlerto properly register the method.Note: The mixing of
#for fields (#dom,#getDynamicStyle) and_for this method is intentional—fields don't require decorator access, while the method does.packages/web-platform/web-elements/src/XText/RawText.ts (1)
20-20: LGTM! Necessary change for decorator compatibility.The rename from
#handleTextto_handleTextis required because the@registerAttributeHandlerdecorator needs runtime access to the method. JavaScript private fields (#) cannot be accessed dynamically, so using underscore convention is the correct approach here. The pattern of keeping fields private with#while making decorated methods conventionally private with_is appropriate.packages/web-platform/web-elements/src/XText/XTextTruncation.ts (2)
299-322: LGTM! Private field renaming enables decorator access.The renaming from
#(truly private) to_(conventionally private) is correct and necessary for the decorators to access these methods. This aligns with enablingnoUnusedLocalsas mentioned in the PR objectives, since decorators need runtime access to these handlers.
386-395: LGTM! Call sites correctly updated.The method invocations have been properly updated to use the new underscore-prefixed names, maintaining consistency with the renamed method definitions.
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (1)
35-35: Verify the visibility change from private#to underscore_prefix.The refactoring changes truly private fields (
#handleEnableTransitionEvent,#enableScrollEventProcessor) to underscore-prefixed public members. This reduces encapsulation guarantees since_members are accessible outside the class, whereas#fields are genuinely private at runtime.This pattern appears intentional for decorator-attached members. If the decorator framework requires dynamic access (via reflection or bracket notation) that's incompatible with private
#fields, this change is justified.Please confirm:
- Is this change required by the
@registerEventEnableStatusChangeHandlerdecorator implementation?- Is this related to the
noUnusedLocalsTypeScript option mentioned in the PR objectives?- Consider documenting why certain members use
_(decorator-accessible) while others retain#(truly private) to clarify the visibility pattern for future maintainers.Also applies to: 173-186
packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts (1)
161-172: Same visibility pattern as XSwiperEvents.ts.The decorator-attached methods
_handleCircularand_handleVerticalChangefollow the same pattern of changing from private#to underscore_prefix. This is consistent with the approach in XSwiperEvents.ts.Also applies to: 195-198
packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts (1)
36-41: Consistent decorator-driven visibility pattern.The methods
_handleCurrentChangeand_handleAutoplayfollow the same visibility change pattern as other XSwiper component files.Also applies to: 62-70
packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (1)
42-47: Consistent decorator-driven visibility pattern for style bindings.All five
bindToStylefield initializers follow the same visibility change pattern, consistent with the other XSwiper component files.Also applies to: 49-54, 56-61, 63-68, 70-75
packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts (3)
25-29: LGTM! Necessary change to support decorators.The rename from
#handleGranularityto_handleGranularityis required because TypeScript/JavaScript decorators cannot access truly private members using#syntax. This change enables the@registerAttributeHandlerdecorator to function correctly.
31-40: LGTM! Necessary change to support decorators.The rename from
#enableOffsetEventto_enableOffsetEventis required for the@registerEventEnableStatusChangeHandlerdecorator to access this method. The implementation correctly manages scroll event listener lifecycle with the passive flag appropriately set.
14-64: Clarification: The refactoring is targeted, not systematic.The AI summary describes this as a "systematic renaming of private class members," but the changes are more targeted: only the two decorated methods (
_handleGranularityand_enableOffsetEvent) are renamed from#to_syntax. Other private members (#dom,#granularity,#pervScroll,#handleScroll) correctly retain the#syntax since they don't need decorator access.This mixed approach is intentional and correct—decorators require accessible members, while truly private fields that don't interact with decorators can remain using
#.packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (1)
81-85: LGTM!Method rename from private
#syntax to underscore-prefixed convention is consistent with the PR's refactoring pattern.packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts (2)
28-62: LGTM! Decorator compatibility improved.The renaming from private
#handleInitialScrollOffsetto_handleInitialScrollOffsetis necessary because JavaScript decorators cannot access truly private (#) class members. This change enables the@registerAttributeHandlerdecorator to function correctly while maintaining the convention that underscore-prefixed methods are internal.
64-81: LGTM! Consistent refactoring applied.The renaming from
#handleInitialScrollIndexto_handleInitialScrollIndexfollows the same pattern as the other handler method, ensuring decorator compatibility throughout the class.packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts (1)
69-69: Document the rationale for renaming decorated methods to_prefix.All six renamed properties are decorated with
@registerEventEnableStatusChangeHandleror@registerAttributeHandler, while all other private members retain the#prefix. This creates a consistent but undocumented pattern where decoration status determines naming convention.This could indicate:
- A workaround for TypeScript's noUnusedLocals incorrectly flagging decorated private members as unused
- A requirement or convention of the decorator system
- An intentional partial refactoring with a specific rationale
Clarify in code comments or commit message why decorated members use
_while other private members use#, or apply the naming convention consistently across the class.packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
48-66: Verify that decorator infrastructure requires public method visibility.The renaming from
#handleComplexEventEnableAttributes(private class field) to_handleComplexEventEnableAttributes(underscore-prefixed public method) changes the visibility from truly private to publicly accessible. While the underscore convention signals internal use, it doesn't enforce encapsulation.Please confirm that the
@registerEventEnableStatusChangeHandlerdecorator requires public accessibility to wire up event handlers. If the decorator can work with private methods, consider reverting to private fields to maintain stronger encapsulation.#!/bin/bash # Description: Check if registerEventEnableStatusChangeHandler decorator implementation # requires public method access or can work with private class fields. # Search for the decorator definition and implementation ast-grep --pattern $'export function registerEventEnableStatusChangeHandler($$$) { $$$ }' # Also search for how it accesses the decorated methods rg -n -A 10 -B 3 "registerEventEnableStatusChangeHandler" --type=ts -g '!**/node_modules/**'packages/web-platform/web-elements/src/XList/XListWaterfall.ts (2)
209-210: Consistent with decorator compatibility pattern.Same reasoning as above - the
@registerAttributeHandlerdecorator requires access to the method descriptor. The change is consistent with the approach applied to the other decorated method in this class.
32-33: Necessary change for decorator compatibility.Method decorators are applied to the Property Descriptor for the method, which requires methods to be accessible via property descriptors. ES private fields (
#) are not exposed through property descriptors, making them incompatible with decorators. Converting only the decorated methods to_prefix—while keeping non-decorated private members as#fields—is the correct approach and aligns with the codebase pattern across XInput, XTextarea, XRefreshView, and other components.packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (2)
74-74: LGTM - visibility change aligns with decorator pattern.The binding logic is correct. The visibility change from
#handleLoopto_handleLoopis consistent with_handleSrcand likely required for the decorator pattern.
83-96: LGTM - event listener management is correct.The conditional event listener registration logic is sound, and using
{ passive: true }is a good performance practice.Note: The mixed privacy syntax (
_handlePauseOnHidevs#documentVisibilitychange) establishes a useful pattern: decorated handler methods use_for tooling compatibility, while helper methods/fields retain true privacy with#.packages/web-platform/web-elements/src/XImage/ImageEvents.ts (2)
22-30: LGTM: Privacy change enables decorator system access.The rename from
#enableLoadEventto_enableLoadEventchanges the method from truly private to conventionally private, which appears intentional to support the@registerEventEnableStatusChangeHandlerdecorator. The logic remains unchanged, and this pattern is consistent with similar changes across other component event classes in the PR.
33-41: LGTM: Consistent with the load event handler pattern.The rename follows the same privacy change pattern as
_enableLoadEvent, maintaining consistency across the event handlers in this class.packages/web-platform/web-worker-rpc/package.json (1)
14-21: LGTM: Modern exports map with source field support.The addition of the
exportsfield with thesourcemapping aligns with the PR objective to support@rsbuild/plugin-source-build. The structure provides proper import/types/default mappings while maintaining backward compatibility through the existingmainandtypingsfields.packages/web-platform/web-elements-reactive/package.json (1)
13-20: LGTM: Consistent exports structure with build tooling support.The exports map follows the same pattern as other packages in this PR, providing proper module resolution and source mapping support. The structure is clean and maintains backward compatibility.
packages/web-platform/web-elements/src/XInput/XInputEvents.ts (1)
34-57: LGTM: Consistent privacy pattern for decorated methods.All three method renames (
_handleEnableInputEvent,_handleSendComposingInput,_handleEnableSelectionEvent) follow the established pattern of using underscore prefix for decorator-accessed methods. The logic remains unchanged, and this aligns with the broader refactoring across event handler classes.Also applies to: 60-62, 130-145
packages/web-platform/web-elements/index.css (1)
6-6: LGTM: Clean CSS consolidation.Replacing multiple individual component CSS imports with a single
./elements.cssimport improves maintainability by centralizing component styles. This aligns with the new CSS aggregation file introduced in the PR.packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts (1)
8-8: LGTM: Appropriate use of type-only import.Converting
WebComponentClassto a type-only import is correct since it's only used in type positions (type assertions on lines 118 and 127). This optimization can help with tree-shaking and makes the import semantics clearer.packages/web-platform/web-elements/elements.css (1)
1-18: LGTM: Comprehensive CSS aggregation.The new
elements.cssfile successfully consolidates all component styles into a single aggregation point, improving maintainability and providing a clear entry point for importing all element styles. The structure is clean and follows standard CSS @import conventions.packages/web-platform/web-elements/package.json (1)
29-29: Format is correct for @rsbuild/plugin-source-build monorepo support.The "source" field additions are properly placed within the exports field following the required structure. The pattern
"source": "./src/<ComponentName>/index.ts"alongside the "default" field is the correct format.However, verify that:
- The @rsbuild/plugin-source-build is registered in your Rsbuild configuration
- TypeScript project references are configured in tsconfig.json to ensure path aliases work correctly if this is a monorepo setup
Also applies to: 34-34, 39-39, 44-44, 49-49, 54-54, 59-59, 64-64, 69-69, 74-74, 79-79, 84-84, 89-89, 94-94, 99-99, 104-104
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
28-61: Change from private (#) to underscore-prefixed (_) fields for decorated handlers is complete and correct.This change enables TypeScript's
noUnusedLocalsoption. The@registerAttributeHandlerdecorator uses field initializers that receive the value directly as a parameter, not reflection-based bracket notation access, so it works equally well with both private and underscore-prefixed fields. The naming convention change addresses TypeScript's type checker, which doesn't recognize private decorated fields as "used" but recognizes underscore-prefixed fields as intentionally exposed. All decorated handlers across the web-elements package consistently use the underscore prefix (0 private decorated fields found), confirming the pattern has been uniformly applied.packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (2)
29-40: LGTM! Good handling of falsy values.The refactored
_handleValuemethod correctly normalizes falsy values to an empty string before updating the input element. This ensures consistent behavior when thevalueattribute is removed or set to null.
43-53: LGTM!The renamed
_handleDisabledand_handleAutocompletebindings maintain the same functionality while following the new underscore-prefix convention for decorator-bound handlers.packages/web-platform/web-elements/src/XTextarea/Placeholder.ts (1)
31-64: LGTM!The placeholder-related handlers have been consistently renamed from private
#syntax to underscore-prefixed identifiers. No behavioral changes; all bindings maintain their original functionality.packages/web-platform/web-elements/src/XText/InlineImage.ts (1)
24-28: LGTM!The
_handleSrcmethod maintains the same behavior as before, correctly setting or removing thesrcattribute based on whether a value is provided.packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
26-39: LGTM!The attribute handlers for
sticky-offsetandspan-count/column-counthave been consistently renamed. The binding logic and value formatters remain unchanged.packages/web-platform/web-elements/src/XSvg/XSvg.ts (3)
27-33: LGTM!The
_handleSrcmethod properly handles both setting and clearing the image source.
36-48: Good improvement: Early return for falsy content.The refactored
_handleContentmethod now includes an early return when content is falsy (lines 38-41), which properly cleans up the URL state and avoids unnecessary blob creation. The URL revocation at line 37 correctly happens before the early return, ensuring proper cleanup.
51-59: LGTM!The
_enableLoadEventhandler maintains proper event listener management with the same passive option configuration.packages/web-platform/web-elements/src/XImage/DropShadow.ts (1)
21-27: The fourth argument tobindToStyleis the existingimportantflag.The
truevalue sets the CSS custom property with!importantpriority. This is a standard pattern used consistently across the codebase (Placeholder.ts, ImageSrc.ts, XSwiper components, etc.) and is not a behavioral change introduced in this PR.packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts (1)
31-31: LGTM: Consistent visibility refactor.The rename from
#handleFadingEdgeLengthand#backgroundColorToVariableto underscore-prefixed equivalents aligns with the broader refactor pattern across the codebase. The handler logic and decorator wiring remain unchanged.Also applies to: 39-39
packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts (1)
34-34: LGTM: Handler visibility updates consistent with refactor.The rename of event handler methods to underscore-prefixed naming maintains the existing event listener logic and decorator registration without behavioral changes.
Also applies to: 60-60, 130-130
packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts (1)
32-32: LGTM: Attribute handler visibility refactor.The underscore-prefixed naming for
_handleEventsPassThroughand_handleVisiblealigns with the broader refactor while preserving the attribute handling logic.Also applies to: 52-52
packages/web-platform/web-elements/src/XImage/ImageSrc.ts (1)
29-29: LGTM: Image attribute handlers refactored consistently.All handler renames follow the underscore convention, and the call site at line 72 is correctly updated to
this._handleSrc(null). The binding logic remains unchanged.Also applies to: 34-34, 41-41, 49-49, 52-52, 72-72
packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts (1)
30-30: LGTM: Textarea base attribute handlers refactored.The underscore-prefixed naming for all five handlers is consistent with the broader refactor. The
bindToAttributelogic and transformations remain unchanged.Also applies to: 40-40, 50-50, 57-57, 64-64
packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (2)
34-34: LGTM: Handler visibility updates.The handler renames for
_handleConfirmEnter,_handleDisabled,_handleMaxHeight, and_handleMinHeightare consistent with the refactor pattern.Also applies to: 39-39, 46-46, 49-49
53-64: The else branch correctly normalizes null/falsy values to empty strings.The native textarea.value property is always a string, which is an empty string if the widget contains no content. The code change aligns with this standard behavior. Since
getAttribute('value')returns null for absent attributes, the else branch (lines 57-59) properly normalizes this to an empty string before assignment. No code in the XTextarea component or its tests relies on distinguishing between null and empty string values—all operations work with strings. This is not a breaking change; it makes the null-to-empty-string coercion explicit and follows standard textarea semantics.packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
33-33: LGTM: Handler visibility refactor.The underscore-prefixed naming for
_handlerConfirmType,_handlerMaxlength,_handleReadonly,_handleType, and_handleSpellCheckaligns with the broader refactor pattern.Also applies to: 43-43, 53-53, 63-91, 95-95
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts
Show resolved
Hide resolved
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-elements/src/XList/XListEvents.ts (1)
355-389: Fix incorrect boolean checks that break event enable/disable logic.Lines 360 and 366 use
!== nullchecks on boolean values from#eventSwitches, which will always evaluate totrue(since bothfalse !== nullandtrue !== nullare true). This causes:
- Line 366: The scroll event listener is attached even when
lynxscrollisfalse(disabled)- Line 360:
#enableScrollEndis always set totrueregardless of the actual enabled stateThe
#eventSwitchesobject stores booleans (initialized at lines 177-183, set at lines 356-357), not nullable values. The checks should test truthiness instead.🔎 Apply this diff to fix the boolean logic:
- this.#enableScrollEnd = lynxscrollend !== null || snap !== null; + this.#enableScrollEnd = lynxscrollend || snap; const listContainer = this.#getListContainer(); // cancel the previous listener first this.#throttledScroll && listContainer.removeEventListener('scroll', this.#throttledScroll); - if (lynxscroll !== null || this.#enableScrollEnd) { + if (lynxscroll || this.#enableScrollEnd) { const wait = scrollEventThrottle !== null ? parseFloat(scrollEventThrottle) : 0;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(8 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)packages/web-platform/web-worker-rpc/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/heavy-birds-beg.md
- packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts
- packages/web-platform/web-worker-rpc/package.json
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
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.
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-elements-reactive/package.jsonpackages/web-platform/web-worker-rpc/src/index.ts
📚 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:
packages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XList/XListEvents.ts
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XList/XListEvents.ts
📚 Learning: 2025-08-29T16:57:19.221Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:24-26
Timestamp: 2025-08-29T16:57:19.221Z
Learning: In the Lynx RPC system, when createRpcEndpoint has hasReturn=true (third parameter), even if the return type is void, it returns a Promise<void> that resolves when the work on the background thread is completed. This allows proper awaiting of cross-thread operations.
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: any)
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: NativeTTObject)
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
🧬 Code graph analysis (1)
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (1)
registerEventEnableStatusChangeHandler(24-24)packages/web-platform/web-elements-reactive/src/registerEventStatusChangedHandler.ts (1)
registerEventEnableStatusChangeHandler(16-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (5)
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (2)
81-85: LGTM! Clean refactoring from private field to underscore prefix.The method rename from
#enableChangeEventto_enableChangeEventaligns with the broader naming convention changes in this PR. The decorator binding and logic remain correct.
87-92: Excellent! Bug fix confirmed—now assigns to the correct flag.Line 90 now correctly assigns
statustothis.#enableOffsetChangeinstead of the incorrectthis.#enableChangethat was flagged in the previous review. This ensures theoffsetchangeevent enable state is tracked properly and the condition on line 95 works as intended.packages/web-platform/web-worker-rpc/src/index.ts (1)
9-11: LGTM! Consistent API extension.The new async RPC endpoint type exports follow the existing naming pattern and properly extend the public API surface. The additions align well with the existing sync variants.
packages/web-platform/web-elements/src/XList/XListEvents.ts (2)
90-90: LGTM: Method visibility refactor is safe.The renaming of methods from hard-private (
#) to underscore-prefixed (_) is consistent with the broader refactoring effort described in the PR. The decorators continue to function correctly with the renamed methods, and based on the past review verification, no external code references these methods.Also applies to: 140-140, 186-186, 236-236, 355-355, 414-414, 428-428
358-358: Past issue resolved:lynxscrollnow properly destructured.The previous review flagged an undefined variable
scrollon line 366. This has been corrected—line 358 now properly includeslynxscrollin the destructuring. However, a new critical issue with the boolean logic on lines 360 and 366 was identified in a separate comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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.
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.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1547
File: packages/rspeedy/core/src/config/loadConfig.ts:11-11
Timestamp: 2025-08-18T08:46:20.001Z
Learning: `#register` and similar imports starting with "#" are Node.js subpath imports defined in the "imports" field of package.json, not TypeScript path mapping aliases. These are natively supported by both Node.js and TypeScript without requiring additional tsconfig.json configuration like "moduleResolution" or "resolvePackageJsonImports" settings.
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts
🔇 Additional comments (1)
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
54-58: Excellent fix for the ResizeObserver reconnection issue!The cleanup logic now properly resets both the observer reference (line 56) and the observing flag (line 57) when disabling. This resolves the critical reconnection failure flagged in the previous review - when the event is re-enabled, the check at line 26 will correctly detect that no observer exists and create a new one.
CodSpeed Performance ReportMerging #2004 will improve performances by 11.18%Comparing Summary
Benchmarks breakdown
Footnotes
|
Web Explorer#6747 Bundle Size — 375.69KiB (+0.82%).4059aa5(current) vs 2ec51b0 main#6744(baseline) Bundle metrics
|
| Current #6747 |
Baseline #6744 |
|
|---|---|---|
 Initial JS |
149.42KiB(+2.09%) |
146.36KiB |
 Initial CSS |
32.4KiB |
32.4KiB |
 Cache Invalidation |
39.28% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
231(+0.43%) |
230 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.03%(+2.02%) |
2.97% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #6747 |
Baseline #6744 |
|
|---|---|---|
| JS |
246.31KiB (+1.26%) |
243.25KiB |
| Other |
96.98KiB |
96.98KiB |
| CSS |
32.4KiB |
32.4KiB |
Bundle analysis report Branch PupilTong:p/hw/deps-support-expo... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#6587 Bundle Size — 236.9KiB (0%).4059aa5(current) vs 2ec51b0 main#6584(baseline) Bundle metrics
|
| Current #6587 |
Baseline #6584 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.74% |
46.74% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6587 |
Baseline #6584 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.14KiB |
91.14KiB |
Bundle analysis report Branch PupilTong:p/hw/deps-support-expo... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
p/hw/deps-support-export-source
branch
from
4764933 to
0e1c277
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
61-65: Ensuredispose()is called during component teardown.The method is now public (previously private), but the concern from the previous review remains valid: consuming components must call
dispose()in theirdisconnectedCallback()to prevent ResizeObserver memory leaks.packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (1)
87-92: LGTM! Bug fix correctly applied.The method rename to
_enableOffsetChangeEventfollows the PR's refactoring pattern, and the assignment at line 90 now correctly updatesthis.#enableOffsetChangeinstead ofthis.#enableChange. This ensures the scroll event listener condition at line 95 properly reflects theoffsetchangeevent's enabled state.packages/web-platform/web-elements-reactive/package.json (1)
13-19: This public package is missing from the changeset.The
exportsfield with source mapping is an API-visible change to this public package, but@lynx-js/web-elements-reactiveis not listed in.changeset/heavy-birds-beg.md. Per project conventions, all public packages with API changes require changeset entries.Note: This issue is already flagged in the review of the changeset file.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (44)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-compat/package.json(1 hunks)packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/elements.css(1 hunks)packages/web-platform/web-elements/index.css(1 hunks)packages/web-platform/web-elements/package.json(1 hunks)packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts(4 hunks)packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts(3 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts(0 hunks)packages/web-platform/web-elements/src/XImage/DropShadow.ts(1 hunks)packages/web-platform/web-elements/src/XImage/ImageEvents.ts(2 hunks)packages/web-platform/web-elements/src/XImage/ImageSrc.ts(2 hunks)packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts(5 hunks)packages/web-platform/web-elements/src/XInput/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XInput/XInputAttribute.ts(2 hunks)packages/web-platform/web-elements/src/XInput/XInputEvents.ts(3 hunks)packages/web-platform/web-elements/src/XList/ListItemAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(8 hunks)packages/web-platform/web-elements/src/XList/XListWaterfall.ts(2 hunks)packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts(2 hunks)packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts(2 hunks)packages/web-platform/web-elements/src/XSvg/XSvg.ts(3 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts(1 hunks)packages/web-platform/web-elements/src/XText/InlineImage.ts(1 hunks)packages/web-platform/web-elements/src/XText/RawText.ts(1 hunks)packages/web-platform/web-elements/src/XText/XTextTruncation.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts(3 hunks)packages/web-platform/web-elements/src/XView/BlurRadius.ts(1 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-elements/tsconfig.json(0 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)packages/web-platform/web-worker-rpc/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts
- packages/web-platform/web-elements/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (21)
- packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts
- packages/web-platform/web-elements/src/XList/ListItemAttributes.ts
- packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts
- packages/web-platform/web-elements/src/XImage/DropShadow.ts
- packages/web-platform/web-elements/src/XList/XListWaterfall.ts
- packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts
- packages/web-platform/web-elements/src/XText/XTextTruncation.ts
- packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts
- packages/web-platform/web-elements/elements.css
- packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts
- packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts
- packages/web-platform/web-elements/src/XText/RawText.ts
- packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts
- packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts
- packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts
- packages/web-platform/web-elements/src/XSvg/XSvg.ts
- packages/web-platform/web-worker-rpc/package.json
- packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts
- packages/web-platform/web-elements/index.css
- packages/web-platform/web-elements/src/XInput/InputBaseAttributes.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/heavy-birds-beg.md
🧠 Learnings (19)
📓 Common learnings
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.
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.
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.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1782
File: packages/react/transform/crates/swc_plugin_inject/napi.rs:31-47
Timestamp: 2025-09-19T07:37:58.778Z
Learning: User gaoachao prefers to keep refactoring PRs minimal and focused, deferring non-essential improvements to separate PRs to maintain clear scope boundaries.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
.changeset/heavy-birds-beg.mdpackages/web-platform/web-elements-reactive/package.json
📚 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/heavy-birds-beg.md
📚 Learning: 2025-09-29T06:43:40.182Z
Learnt from: CR
Repo: lynx-family/lynx-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T06:43:40.182Z
Learning: Applies to .changeset/*.md : For contributions, generate and commit a Changeset describing your changes
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
.changeset/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 Learning: 2025-08-29T16:57:19.221Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:24-26
Timestamp: 2025-08-29T16:57:19.221Z
Learning: In the Lynx RPC system, when createRpcEndpoint has hasReturn=true (third parameter), even if the return type is void, it returns a Promise<void> that resolves when the work on the background thread is completed. This allows proper awaiting of cross-thread operations.
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: NativeTTObject)
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: any)
Applied to files:
packages/web-platform/web-worker-rpc/src/index.ts
📚 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-elements/src/XSwiper/XSwiperEvents.tspackages/web-platform/web-elements/src/XInput/XInputEvents.ts
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements/src/XList/XListAttributes.ts
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.tspackages/web-platform/web-elements/src/XList/XListEvents.tspackages/web-platform/web-elements/src/XInput/XInputAttribute.tspackages/web-platform/web-elements/src/XText/InlineImage.ts
📚 Learning: 2025-11-11T08:05:14.163Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.163Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/XInput/XInputAttribute.ts
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XList/XListEvents.ts
🧬 Code graph analysis (5)
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (2)
packages/web-platform/web-elements-reactive/src/bindSwitchToEventListener.ts (1)
bindSwitchToEventListener(8-32)packages/web-platform/web-elements-reactive/src/index.ts (1)
bindSwitchToEventListener(5-5)
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (1)
registerEventEnableStatusChangeHandler(24-24)packages/web-platform/web-elements-reactive/src/registerEventStatusChangedHandler.ts (1)
registerEventEnableStatusChangeHandler(16-24)
packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
🔇 Additional comments (26)
packages/web-platform/web-elements/src/XView/BlurRadius.ts (1)
23-32: LGTM!The rename from
#handleBlurRadiusto_handleBlurRadiusis appropriate. ES private methods can have compatibility issues with decorators that rely on property descriptors, so using the underscore convention for decorated methods while keeping internal state fields (#dom,#getDynamicStyle) truly private is a sensible approach.packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
56-57: Good defensive cleanup when event is disabled.Clearing the observer reference and resetting the flag when the layoutchange event is disabled prevents resource accumulation if the event is toggled multiple times during the component lifecycle.
packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (1)
81-85: LGTM!The method rename from
#enableChangeEventto_enableChangeEventfollows the PR's broader refactoring pattern for decorator compatibility with the reactive framework. The logic correctly updates#enableChangeand triggers the scroll listener setup.packages/web-platform/web-elements/src/XTextarea/Placeholder.ts (1)
31-64: LGTM! Clean refactor to support decorator access.The change from
#(true private) to_(convention-based private) for the decorated handler methods is correct. The@registerAttributeHandlerdecorator and reactive framework need to access these members, which true private fields would prevent. The distinction between_for decorated handlers and#for internal implementation details (#getTextarea,#dom) is clear and appropriate.packages/web-platform/web-worker-rpc/src/index.ts (1)
9-11: All verified. The three new async RPC endpoint types (RpcEndpointAsync,RpcEndpointAsyncVoid,RpcEndpointAsyncWithTransfer) are properly defined inRpcEndpoint.tsand actively used throughout the codebase in type constraints, method overloads, and utility types withinTypeUtils.ts,Rpc.ts, andRpcEndpoint.tsitself. These are legitimate public API additions for the package and will not trigger unused variable warnings withnoUnusedLocalsenabled.packages/web-platform/web-elements/src/XText/InlineImage.ts (1)
25-28: Rename from private to underscore-convention looks good.The method rename from
#handleSrcto_handleSrcfollows the consistent pattern across this PR. The decorator binding and logic remain unchanged.packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts (1)
161-172: Attribute handler renames are consistent.Both
_handleCircularand_handleVerticalChangefollow the underscore-convention renaming pattern. The decorator bindings and internal logic remain unchanged.Also applies to: 195-198
packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
27-31: Field renames maintain binding behavior.The renames
_handlerStickyOffsetand_handlerCountfollow the PR's consistent pattern. Both still correctly bind to CSS custom properties viabindToStyle.Also applies to: 35-39
packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts (1)
30-68: Multiple handler renames follow consistent pattern.All five attribute handlers (
_handlerConfirmType,_handlerMaxlength,_handleReadonly,_handleSpellCheck,_handleShowSoftInputOnfocus) adopt the underscore convention. ThebindToAttributebindings and transformation logic remain intact.packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (2)
29-40: Behavioral change: falsy values now normalized to empty string.Beyond the rename from
#handleValueto_handleValue, this change adds anelsebranch (lines 33-35) that explicitly setsnewValue = ''when the input is falsy. Previously, falsy values would be passed directly toinput.value, which could result in setting the input tonullorundefined.This normalization ensures the input always receives a string value, which is more defensive and aligns with HTML input behavior.
43-53: Handler renames look good.Both
_handleDisabledand_handleAutocompletefollow the consistent underscore-convention pattern with preservedbindToAttributebehavior.packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (1)
35-40: Event handler renames preserve functionality.Both
_handleEnableTransitionEventand_enableScrollEventProcessoradopt the underscore convention while maintaining their decorator bindings and event management logic.Also applies to: 173-186
packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts (1)
32-49: Overlay attribute handlers renamed consistently.Both
_handleEventsPassThroughand_handleVisiblefollow the underscore-convention pattern. The event pass-through logic and dialog visibility management remain unchanged.Also applies to: 52-67
packages/web-platform/web-elements/src/XImage/ImageSrc.ts (1)
29-53: Image attribute handlers renamed with call site updated.All five handlers (
_handleSrc,_preloadPlaceholder,_handleBlurRadius,_handleCrossorigin,_handleReferrerpolicy) adopt the underscore convention. The call site on line 72 correctly references the renamed_handleSrcmethod.Also applies to: 72-72
packages/web-platform/web-elements/src/XList/XListEvents.ts (7)
90-137: LGTM! Method visibility change is consistent with refactor pattern.The rename from hard-private to soft-private is intentional and aligns with the broader refactor across XList components. The decorator
@registerEventEnableStatusChangeHandlerworks correctly with the underscore-prefixed method name.
140-163: LGTM! Attribute handler correctly renamed.The decorator
@registerAttributeHandlerfunctions properly with the new method name. The logic for handling upper threshold item count changes remains correct.
186-233: LGTM! ScrollToLower event handler properly refactored.The method correctly manages the lower scroll event switches and observer lifecycle. The decorator integration remains functional with the new naming convention.
236-258: LGTM! Lower threshold handler correctly updated.The attribute handler for lower threshold item count changes is properly implemented with the new method naming convention.
355-389: LGTM! Previous critical issue resolved and refactor completed correctly.The method now correctly destructures
lynxscrollat line 358 and references it properly in the condition at line 366, fixing the ReferenceError from the previous review. The multiple decorator registrations work correctly with the renamed method.
414-419: LGTM! Upper edge event handler properly renamed.The event handler for scroll-to-upper-edge functionality is correctly implemented with the new naming convention.
428-433: LGTM! Lower edge event handler properly renamed.The event handler for scroll-to-lower-edge functionality is correctly implemented with the new naming convention.
packages/web-platform/web-elements/src/XImage/ImageEvents.ts (2)
32-41: Consistent refactoring applied.The rename from
enableErrorEventto_enableErrorEventis consistent with the project-wide naming convention for internal event handler methods.
21-30: The method rename fromenableLoadEventto_enableLoadEventis safe and requires no further verification. The@registerEventEnableStatusChangeHandlerdecorator stores handlers by event key (e.g., 'load', 'error'), not by method name, so the naming change has no impact on runtime behavior or decorator functionality.packages/web-platform/web-elements-compat/package.json (1)
18-20: LGTM! Wildcard export enables flexible subpath imports.The
./LinearContainer/*wildcard export allows users to import any subpath under LinearContainer (e.g.,@lynx-js/web-elements-compat/LinearContainer/SomeSubComponent). This expands the public API surface while maintaining consistency with the existing export structure.packages/web-platform/web-elements/package.json (1)
21-23: LGTM! CSS export and source mappings properly configured.The additions are well-structured:
- The new
./elements.cssexport enables users to import consolidated component styles- The
sourcefields on all component exports enable @rsbuild/plugin-source-build integration- All source paths correctly point to the corresponding TypeScript entry files
The package is already included in the changeset, though the changeset description could better highlight the new CSS export (addressed in the changeset file review).
Also applies to: 29-29, 34-34, 39-39, 44-44, 49-49, 54-54, 59-59, 64-64, 69-69, 74-74, 79-79, 84-84, 89-89, 94-94, 99-99, 104-104
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
28-61: The refactor from#to_for decorated fields is correct and necessary.The change ensures the
@registerAttributeHandlerdecorator can properly intercept and initialize these handler fields. The decorator implementation wraps each decorated field's initial value to register it in an internal handler registry. This field initialization interception mechanism requires public field semantics and works reliably with_-prefixed fields, whereas private#fields have different transpilation semantics that can interfere with the decorator's initialization hook.The selective nature of this change—keeping
#domand#getInputElementas truly private while only exposing the@registerAttributeHandler-decorated fields—demonstrates this is a targeted fix for decorator compatibility, not a general API surface expansion.No further action needed.
PupilTong
force-pushed
the
p/hw/deps-support-export-source
branch
from
0e1c277 to
80e8ac5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web-platform/web-elements-reactive/package.json (1)
13-19: Verify changeset file exists for this public package change.The "exports" field structure is technically correct and properly implements the "source" mapping for @rsbuild/plugin-source-build support. However, a changeset should be added for this public package to document the change and manage versioning.
Run the following script to verify if a changeset was added:
#!/bin/bash # Check if a changeset exists for @lynx-js/web-elements-reactive echo "=== Searching for changesets mentioning @lynx-js/web-elements-reactive ===" if [ -d ".changeset" ]; then # Search for changeset files mentioning this package grep -r "@lynx-js/web-elements-reactive" .changeset/*.md 2>/dev/null | head -10 if [ $? -eq 0 ]; then echo -e "\n✓ Changeset found for @lynx-js/web-elements-reactive" else echo -e "\n✗ No changeset found mentioning @lynx-js/web-elements-reactive" echo "Consider adding a changeset to document this export structure change." fi echo -e "\n=== Recent changesets in this PR branch ===" git log origin/main..HEAD --oneline --name-only -- '.changeset/*.md' | head -20 else echo "✗ .changeset directory not found" fi
🧹 Nitpick comments (1)
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
54-59: Consider extracting the cleanup logic to reduce duplication.The cleanup logic on lines 56-57 is duplicated in the
dispose()method (lines 63-64). Consider extracting to a private helper method.🔎 View suggested refactor
+ #clearObserver() { + this.#resizeObserver?.disconnect(); + this.#resizeObserver = undefined; + this.#resizeObserving = false; + } + @registerEventEnableStatusChangeHandler('layoutchange') __handleScrollUpperThresholdEventEnabled = (enabled: boolean) => { if (enabled && this.#dom[layoutChangeTarget]) { // ... observer setup code ... } else { - this.#resizeObserver?.disconnect(); - this.#resizeObserver = undefined; - this.#resizeObserving = false; + this.#clearObserver(); } }; dispose() { - this.#resizeObserver?.disconnect(); - this.#resizeObserver = undefined; - this.#resizeObserving = false; + this.#clearObserver(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (44)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-compat/package.json(1 hunks)packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/elements.css(1 hunks)packages/web-platform/web-elements/index.css(1 hunks)packages/web-platform/web-elements/package.json(1 hunks)packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts(4 hunks)packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts(3 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts(0 hunks)packages/web-platform/web-elements/src/XImage/DropShadow.ts(1 hunks)packages/web-platform/web-elements/src/XImage/ImageEvents.ts(2 hunks)packages/web-platform/web-elements/src/XImage/ImageSrc.ts(2 hunks)packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts(5 hunks)packages/web-platform/web-elements/src/XInput/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XInput/XInputAttribute.ts(2 hunks)packages/web-platform/web-elements/src/XInput/XInputEvents.ts(3 hunks)packages/web-platform/web-elements/src/XList/ListItemAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(8 hunks)packages/web-platform/web-elements/src/XList/XListWaterfall.ts(2 hunks)packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts(2 hunks)packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts(2 hunks)packages/web-platform/web-elements/src/XSvg/XSvg.ts(3 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts(1 hunks)packages/web-platform/web-elements/src/XText/InlineImage.ts(1 hunks)packages/web-platform/web-elements/src/XText/RawText.ts(1 hunks)packages/web-platform/web-elements/src/XText/XTextTruncation.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts(3 hunks)packages/web-platform/web-elements/src/XView/BlurRadius.ts(1 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-elements/tsconfig.json(0 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)packages/web-platform/web-worker-rpc/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts
- packages/web-platform/web-elements/tsconfig.json
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- .changeset/heavy-birds-beg.md
- packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts
- packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts
- packages/web-platform/web-elements/src/XText/RawText.ts
- packages/web-platform/web-elements-compat/package.json
- packages/web-platform/web-elements/src/XImage/DropShadow.ts
- packages/web-platform/web-elements/src/XList/XListWaterfall.ts
- packages/web-platform/web-worker-rpc/src/index.ts
- packages/web-platform/web-elements/index.css
- packages/web-platform/web-elements/elements.css
- packages/web-platform/web-elements/package.json
- packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts
- packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts
- packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts
- packages/web-platform/web-elements/src/XView/BlurRadius.ts
- packages/web-platform/web-elements/src/XList/XListEvents.ts
- packages/web-platform/web-elements/src/XSvg/XSvg.ts
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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.
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.
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.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1782
File: packages/react/transform/crates/swc_plugin_inject/napi.rs:31-47
Timestamp: 2025-09-19T07:37:58.778Z
Learning: User gaoachao prefers to keep refactoring PRs minimal and focused, deferring non-essential improvements to separate PRs to maintain clear scope boundaries.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
📚 Learning: 2025-11-11T08:05:14.163Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.163Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/XInput/XInputAttribute.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts
📚 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-elements/src/XInput/XInputEvents.ts
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.tspackages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XList/XListAttributes.ts
📚 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-elements/src/XInput/XInputEvents.ts
📚 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:
packages/web-platform/web-elements/src/XInput/XInputEvents.tspackages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.tspackages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.tspackages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XText/XTextTruncation.tspackages/web-platform/web-elements/src/XText/InlineImage.tspackages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.tspackages/web-platform/web-elements/src/XInput/XInputAttribute.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.tspackages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-elements-reactive/package.jsonpackages/web-platform/web-worker-rpc/package.json
📚 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:
packages/web-platform/web-elements-reactive/package.json
📚 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:
packages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
packages/web-platform/web-elements-reactive/package.json
📚 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:
packages/web-platform/web-elements-reactive/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-elements-reactive/package.jsonpackages/web-platform/web-worker-rpc/package.json
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts
🧬 Code graph analysis (11)
packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToStyle(8-8)registerAttributeHandler(20-20)packages/web-platform/web-elements-reactive/src/registerAttributeHandler.ts (1)
registerAttributeHandler(19-30)
packages/web-platform/web-elements/src/XList/ListItemAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts (3)
packages/web-platform/web-elements-reactive/src/index.ts (3)
bindToStyle(8-8)registerAttributeHandler(20-20)registerEventEnableStatusChangeHandler(24-24)packages/web-platform/web-elements-reactive/src/registerAttributeHandler.ts (1)
registerAttributeHandler(19-30)packages/web-platform/web-elements-reactive/src/registerEventStatusChangedHandler.ts (1)
registerEventEnableStatusChangeHandler(16-24)
packages/web-platform/web-elements/src/XImage/ImageSrc.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
packages/web-platform/web-elements/src/XTextarea/Placeholder.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToStyle(8-8)bindToAttribute(6-6)
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (2)
packages/web-platform/web-elements-reactive/src/bindSwitchToEventListener.ts (1)
bindSwitchToEventListener(8-32)packages/web-platform/web-elements-reactive/src/index.ts (1)
bindSwitchToEventListener(5-5)
packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / rustfmt
- GitHub Check: test-rust / clippy
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: code-style-check
🔇 Additional comments (27)
packages/web-platform/web-elements/src/XText/InlineImage.ts (1)
25-28: LGTM! Visibility change aligns with decorator requirements.The rename from
#handleSrcto_handleSrcis necessary because decorators cannot be applied to private (#) class members. The attribute-handling logic remains correct and unchanged.packages/web-platform/web-elements/src/XList/ListItemAttributes.ts (1)
23-27: LGTM! Field rename aligns with refactor pattern.The change from
#handlerEstimatedMainAxisSizePxto_handlerEstimatedMainAxisSizePxis part of the broader refactor pattern mentioned in the PR summary. This visibility change (from truly private#to convention-based_) is likely necessary for the@registerAttributeHandlerdecorator and reactive system to programmatically access the field, as JavaScript decorators cannot directly access private#fields.packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (1)
42-75: LGTM: Consistent refactoring pattern for decorator compatibility.The systematic renaming of decorated handler fields from private (
#) to underscore-prefixed (_) is correct and maintains consistency. This pattern allows the@registerAttributeHandlerdecorator to access these properties at runtime, which JavaScript private fields would prevent.The selective approach—keeping truly internal fields like
#dom,#numOfChildElement, etc. as private while exposing only the decorated handlers—strikes a reasonable balance between encapsulation and framework requirements.packages/web-platform/web-elements/src/XInput/XInputEvents.ts (1)
34-34: Visibility change from truly-private to conventionally-private is consistent.The three decorated handler methods are consistently renamed from hash-private (
#) to underscore-prefixed (_), changing them from ES2022 truly-private to conventionally-private public members. This reduces encapsulation but aligns with the PR's broader refactoring pattern across the codebase. All other private members (#dom,#sendComposingInput,#getInputElement, etc.) correctly retain hash-private syntax, and method logic remains unchanged.As noted in the previous review, the reasoning about decorator compatibility has been clarified—this change is for semantic consistency rather than technical necessity.
Also applies to: 60-60, 130-130
packages/web-platform/web-elements/src/XText/XTextTruncation.ts (1)
302-302: LGTM! Decorator compatibility requires public method access.The change from
#(truly private) to_(public by convention) is necessary for the decorators@registerAttributeHandlerand@registerEventEnableStatusChangeHandlerto function correctly. ES2022+ truly private fields cannot be accessed via prototype chains or through decorator metadata, so this refactor enables the reactive attribute system to properly register and invoke these handlers.The underscore prefix appropriately signals these methods remain internal implementation details despite being technically public.
Also applies to: 320-320, 388-391
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (2)
35-35: LGTM! Visibility change necessary for decorator access.The change from
#handleEnableTransitionEventto_handleEnableTransitionEventis required because TypeScript/JavaScript decorators cannot access private fields (prefixed with#). The underscore prefix maintains the convention that this member is internal.
173-173: LGTM! Visibility change necessary for decorator access.The change from
#enableScrollEventProcessorto_enableScrollEventProcessoris required for the multiple@registerEventEnableStatusChangeHandlerdecorators to access this method. The underscore prefix maintains the internal/private convention.packages/web-platform/web-worker-rpc/package.json (1)
14-20: Exports field correctly configured for @rsbuild/plugin-source-buildThe exports field with the "source" field pointing to the source code and "default" to the built output follows the documented pattern for @rsbuild/plugin-source-build. The "types" field placement and backward compatibility with the existing "main" and "typings" fields (lines 21-22) are well-structured for modern Node.js tooling.
packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (2)
33-38: Improved error handling with safe fallback.The try-catch wrapper around JSON.parse prevents runtime crashes from malformed JSON. Defaulting to
{}on parse error ensures the subsequent validation logic (lines 52-66) properly dispatches aSrcJsonErrorevent, which is the correct behavior.
31-31: Acknowledge this is an intentional refactoring pattern with minimal runtime risk.The change from truly private (
#) to conventionally private (_) handler methods is intentional and part of a broader refactor pattern across the web-elements package. Since these methods are framework-invoked via the@registerAttributeHandlerdecorator—not called directly from external code—the visibility change has minimal practical impact. The reactive framework mechanism works equivalently with both approaches.packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts (2)
89-92: Bug fix applied correctly—offsetchange event tracking now works.The assignment on line 90 now correctly updates
this.#enableOffsetChangeinstead of the wrong flag (this.#enableChange). This fix ensures that the condition in#enableScrollEventListener()(line 95) properly reflects whetheroffsetchangeevents are enabled.This addresses the critical bug identified in the previous review. ✅
82-85: No verification needed for decorator compatibility claim.The visibility change from
#enableChangeEventto_enableChangeEventis not required by@registerEventEnableStatusChangeHandler. The decorator implementation supports both private and public methods through the standard Stage 3 decorator specification, which provides context information about member visibility without requiring public access. The change appears to be a code pattern/style refactoring rather than a decorator compatibility requirement.Likely an incorrect or invalid review comment.
packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts (1)
69-69: LGTM: Pure refactoring to underscore-prefixed naming.The rename of event and attribute handlers from private
#to public_prefix maintains all decorator bindings and logic. This aligns with the broader refactoring pattern across the codebase.Also applies to: 83-83, 97-97, 104-104, 160-160, 167-167
packages/web-platform/web-elements/src/XImage/ImageEvents.ts (1)
22-22: LGTM: Consistent refactoring to public naming convention.The event handler renames maintain decorator bindings and logic.
Also applies to: 33-33
packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
27-27: LGTM: Attribute handler refactoring.Field renames preserve the
bindToStylebindings and decorator configurations.Also applies to: 35-35
packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts (1)
26-26: LGTM: Handler naming refactor.The granularity and offset event handlers maintain their decorator bindings.
Also applies to: 32-32
packages/web-platform/web-elements/src/XInput/XInputAttribute.ts (2)
43-47: LGTM: Handler renames maintain bindings.The
disabledandautocompletehandler renames preserve theirbindToAttributeconfigurations.Also applies to: 50-53
29-40: The concern about breaking existing code expecting null/undefined is unfounded.Converting falsy values to an empty string is the correct and safe approach for
input.value. Per the DOM standard,HTMLInputElement.valueis always a string property—it cannot be null or undefined. Setting it to null, undefined, or any other falsy value automatically results in an empty string. Code reading frominput.valuewill always receive a string, so no existing code should expect null/undefined behavior from this property. This implementation is defensive and correct, not problematic.packages/web-platform/web-elements/src/XImage/ImageSrc.ts (1)
29-29: LGTM: Comprehensive handler refactoring with updated call site.All handler renames maintain their decorator bindings and binding configurations. The
connectedCallbackproperly uses the renamed_handleSrcmethod (line 72).Also applies to: 34-38, 41-46, 49-49, 52-52, 72-72
packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts (1)
31-35: LGTM: Style handler refactoring.The fading edge length and background color handlers maintain their decorator bindings and style update logic.
Also applies to: 39-48
packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
51-51: LGTM! Consistent decorator-friendly naming.The refactoring from hash-prefixed private fields to underscore-prefixed fields for decorated event handlers improves compatibility with the decorator runtime. The pattern is consistent: decorated handlers use underscore prefix while internal state and non-decorated helpers remain truly private with hash prefix.
Also applies to: 72-72
packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (2)
34-34: LGTM! Consistent refactoring.The handler renaming from hash-prefixed to underscore-prefixed follows the same decorator-friendly pattern seen across the codebase.
Also applies to: 39-43, 46-46, 49-49
53-64: Good improvement to value handling.The added else branch at lines 57-58 ensures
newValueis always normalized to an empty string when null/undefined, making the behavior more explicit. While DOM APIs coerce null to empty string anyway, this improves code clarity.packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
29-29: LGTM! Consistent decorator pattern.All placeholder-related handlers correctly refactored to underscore-prefixed fields while maintaining identical binding logic.
Also applies to: 32-37, 40-45, 48-53, 56-61
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
33-40: LGTM! Well-structured refactoring.The changes maintain a clear separation: decorated attribute handlers use underscore prefix (e.g.,
_handleType,_handlerConfirmType) while internal binding helpers remain truly private with hash prefix (e.g.,#setType,#setInputmode). The new#setTypebinding at line 59 correctly follows this pattern.Also applies to: 43-50, 53-57, 59-91, 95-99
packages/web-platform/web-elements/src/XTextarea/Placeholder.ts (1)
32-37: LGTM! Consistent with XInput placeholder pattern.The handler refactoring mirrors the approach in
XInput/Placeholder.ts, maintaining consistency across textarea and input components.Also applies to: 40-45, 48-53, 56-61, 64-64
packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts (1)
34-57: LGTM! Decorator-compatible event handlers.The three refactored methods (
_handleEnableConfirmEvent,_handleSendComposingInput,_handleEnableSelectionEvent) correctly adopt underscore prefix as they are decorated, while internal event handlers like#teleportEventand#teleportInputappropriately remain truly private.Also applies to: 60-62, 130-145
PupilTong
force-pushed
the
p/hw/deps-support-export-source
branch
from
80e8ac5 to
f578474
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
61-65: The disposal method is still not invoked by consuming components.As noted in the previous review, while the
dispose()implementation is correct, it remains unused across the codebase. The method must be called fromdisconnectedCallback()in all components that useCommonEventsAndMethodsto prevent memory leaks from uncleaned ResizeObservers..changeset/heavy-birds-beg.md (1)
1-11: The previous review comment about the incomplete changeset remains unaddressed.The changeset still omits
@lynx-js/web-elements-reactiveand lacks clarity on user-facing changes (new./elements.cssexport and./LinearContainer/*wildcard export). Per project conventions, all public packages with API changes require changeset entries.packages/web-platform/web-elements/package.json (1)
21-23: New CSS export is correctly structured.The
./elements.cssexport is properly configured. The documentation issue (missing from changeset) is already addressed in the changeset file review.
🧹 Nitpick comments (1)
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
29-61: Refactor aligns with enablingnoUnusedLocalscompiler option.The change from private fields (
#handler*) to public fields with underscore prefix (_handler*) is an appropriate workaround for TypeScript'snoUnusedLocalslimitation with decorated fields. The decorator system uses these fields, but the compiler may not recognize this usage, resulting in false-positive "unused" warnings. The underscore prefix correctly signals these are internal implementation details.💡 Optional: Add explanatory comment
Consider adding a brief comment above the first handler declaration to document this decision:
#getInputElement = genDomGetter(() => this.#dom.shadowRoot!, '#input'); + // Note: Handler fields use _ prefix instead of # to avoid false positives + // with noUnusedLocals when fields are accessed only via decorators. @registerAttributeHandler('placeholder', true) _handlerPlaceholder = bindToAttribute(this.#getInputElement, 'placeholder');This helps future maintainers understand why these fields are public rather than truly private.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-compat/package.json(1 hunks)packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/elements.css(1 hunks)packages/web-platform/web-elements/index.css(1 hunks)packages/web-platform/web-elements/package.json(1 hunks)packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts(4 hunks)packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts(3 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts(0 hunks)packages/web-platform/web-elements/src/XImage/DropShadow.ts(1 hunks)packages/web-platform/web-elements/src/XImage/ImageEvents.ts(2 hunks)packages/web-platform/web-elements/src/XImage/ImageSrc.ts(2 hunks)packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts(5 hunks)packages/web-platform/web-elements/src/XInput/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XInput/XInputAttribute.ts(2 hunks)packages/web-platform/web-elements/src/XInput/XInputEvents.ts(3 hunks)packages/web-platform/web-elements/src/XList/ListItemAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(8 hunks)packages/web-platform/web-elements/src/XList/XListWaterfall.ts(2 hunks)packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts(2 hunks)packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts(2 hunks)packages/web-platform/web-elements/src/XSvg/XSvg.ts(3 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts(1 hunks)packages/web-platform/web-elements/src/XText/InlineImage.ts(1 hunks)packages/web-platform/web-elements/src/XText/RawText.ts(1 hunks)packages/web-platform/web-elements/src/XText/XTextTruncation.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts(3 hunks)packages/web-platform/web-elements/src/XView/BlurRadius.ts(1 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-elements/tsconfig.json(0 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)packages/web-platform/web-worker-rpc/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts
- packages/web-platform/web-elements/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/web-platform/web-elements-reactive/package.json
- packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts
- packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts
- packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts
- packages/web-platform/web-worker-rpc/package.json
- packages/web-platform/web-elements-compat/package.json
- packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts
- packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts
- packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts
- packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts
- packages/web-platform/web-elements/elements.css
- packages/web-platform/web-elements/src/XInput/XInputEvents.ts
- packages/web-platform/web-elements/src/XImage/ImageSrc.ts
- packages/web-platform/web-elements/src/XText/InlineImage.ts
- packages/web-platform/web-elements/src/XList/XListWaterfall.ts
- packages/web-platform/web-elements/src/XInput/XInputAttribute.ts
- packages/web-platform/web-elements/index.css
- packages/web-platform/web-elements/src/XTextarea/Placeholder.ts
- packages/web-platform/web-elements/src/XImage/DropShadow.ts
- packages/web-platform/web-elements/src/XSvg/XSvg.ts
- packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts
- packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts
- packages/web-platform/web-elements/src/XView/BlurRadius.ts
- packages/web-platform/web-worker-rpc/src/index.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/heavy-birds-beg.md
🧠 Learnings (19)
📓 Common learnings
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.
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.
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.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XText/RawText.tspackages/web-platform/web-elements/src/XText/XTextTruncation.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/ScrollView/ScrollAttributes.tspackages/web-platform/web-elements/package.jsonpackages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.tspackages/web-platform/web-elements/src/XList/XListEvents.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.tspackages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/src/XList/ListItemAttributes.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 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/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
.changeset/heavy-birds-beg.md
📚 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/heavy-birds-beg.md
📚 Learning: 2025-09-29T06:43:40.182Z
Learnt from: CR
Repo: lynx-family/lynx-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T06:43:40.182Z
Learning: Applies to .changeset/*.md : For contributions, generate and commit a Changeset describing your changes
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-11-06T01:19:23.670Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1917
File: packages/mcp-servers/devtool-mcp-server/tsconfig.json:8-8
Timestamp: 2025-11-06T01:19:23.670Z
Learning: The lynx-js/devtool-mcp-server package in lynx-family/lynx-stack targets Node.js >=18.19 (specified in its package.json engines), which is different from the root project's requirement of Node.js ^22 || ^24. The package uses "lib": ["ES2024.Promise"] in its tsconfig.json because it manually includes polyfills for Promise.withResolvers while maintaining compatibility with Node.js v18.
Applied to files:
.changeset/heavy-birds-beg.md
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
.changeset/heavy-birds-beg.mdpackages/web-platform/web-elements/package.json
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-elements/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-elements/package.json
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XList/XListEvents.ts
📚 Learning: 2025-11-11T08:05:14.163Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.163Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts
🧬 Code graph analysis (4)
packages/web-platform/web-elements/src/XList/ListItemAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToStyle(8-8)
packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
packages/web-platform/web-elements/src/XInput/Placeholder.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / clippy
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (24)
packages/web-platform/web-elements/src/XList/ListItemAttributes.ts (1)
23-27: LGTM! Field rename is consistent with the broader refactoring effort.The rename from
#handlerEstimatedMainAxisSizePxto_handlerEstimatedMainAxisSizePxchanges the field from truly private (using JavaScript's#syntax) to conventionally private (using underscore prefix). This is part of the project-wide refactoring mentioned in the PR summary and doesn't affect the binding logic or behavior.packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
27-31: LGTM! Field renames are consistent with the refactoring pattern.Both field renames (
#handlerStickyOffset→_handlerStickyOffsetand#handlerCount→_handlerCount) follow the same naming convention change from truly private to conventionally private fields. The binding logic and decorators remain unchanged, preserving all existing behavior.Also applies to: 35-39
packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (3)
31-71: Good defensive improvement with try-catch for JSON parsing.The added error handling around
JSON.parseprevents potential uncaught exceptions whennewValuecontains malformed JSON. The fallback to an empty object ensures the subsequent validation logic correctly dispatches appropriate error events (SrcErrorfor null,SrcJsonErrorfor missing fields).
73-74: LGTM!Straightforward rename consistent with the other handler methods.
82-96: LGTM!The event listener lifecycle management is correct—adding on attribute set and removing on attribute removal.
packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts (1)
56-57: Good cleanup consistency improvement.Adding explicit clearing of the observer reference and resetting of the observing flag in the event-disable path ensures proper cleanup and allows clean re-initialization if the event is re-enabled later.
packages/web-platform/web-elements/src/XText/RawText.ts (1)
19-20: Verify the decorator compatibility rationale.The method visibility has changed from truly private (
#handleText) to conventionally private (_handleText). This weakens runtime encapsulation but may be necessary for the@registerAttributeHandlerdecorator to function properly.Please confirm that the
@lynx-js/web-elements-reactiveframework's@registerAttributeHandlerdecorator requires public method access, as native private fields (#) can have compatibility issues with TypeScript decorators:#!/bin/bash # Description: Check if the framework documentation or other attribute handlers use underscore-prefixed methods with @registerAttributeHandler # Search for other uses of @registerAttributeHandler to see the pattern rg -n -A 2 '@registerAttributeHandler' --type ts | head -50packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts (1)
8-8: LGTM! Type-only import is appropriate.Since
WebComponentClassis only used in type positions (lines 118, 127), converting it to a type-only import correctly signals compile-time-only usage and improves tree-shaking.packages/web-platform/web-elements/package.json (1)
29-104: LGTM! Source field additions are correctly structured.The
"source"fields consistently map to the corresponding TypeScript source files and correctly support@rsbuild/plugin-source-buildas stated in the PR objectives.packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts (2)
31-62: LGTM! Clean refactor from hard-private to soft-private naming.The rename from
#handleInitialScrollOffsetto_handleInitialScrollOffsetmaintains all functionality and decorator bindings. The logic for handling scroll offsets remains correct.
66-81: LGTM! Consistent naming pattern applied.The rename from
#handleInitialScrollIndexto_handleInitialScrollIndexis consistent with the broader refactoring pattern and preserves the scroll-to-index functionality.packages/web-platform/web-elements/src/XImage/ImageEvents.ts (1)
22-41: LGTM! Event handler visibility refactor is correct.The rename of both
#enableLoadEventto_enableLoadEventand#enableErrorEventto_enableErrorEventmaintains the event listener registration/deregistration logic correctly. The decorator bindings are properly updated.packages/web-platform/web-elements/src/XText/XTextTruncation.ts (2)
302-322: LGTM! Attribute and event handler renames applied correctly.The methods
_handleAttributeChangeand_handleEnableLayoutEventare properly renamed with their decorator bindings intact. The text truncation logic remains unchanged.
386-395: LGTM! Call sites updated to match renamed methods.The
connectedCallbackcorrectly invokes_handleEnableLayoutEventand_handleAttributeChangewith the new underscore-prefixed names, maintaining the initialization flow.packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (2)
34-49: LGTM! Handler renames and bindings are correct.The refactor from
#handleConfirmEnter,#handleDisabled,#handleMaxHeight, and#handleMinHeightto their underscore-prefixed equivalents is consistent. ThebindToAttributeandbindToStylebindings remain properly configured.
53-64: LGTM! Improved null handling in value setter.The addition of the else branch (lines 57-58) to set
newValue = ''when null is a good improvement. This ensurestextarea.valuealways receives a string rather than null, which is more correct for the HTMLTextAreaElement value property.packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (2)
33-58: LGTM! Consistent handler renames with proper bindings.The rename of
#handlerConfirmType,#handlerMaxlength, and#handleReadonlyto underscore-prefixed versions maintains all thebindToAttributelogic correctly. The decorators are properly updated.
59-91: LGTM! Improved type handling with binding pattern.The refactor introduces
#setType = bindToAttribute(...)(line 59) and uses it in_handleType(line 90), which is more consistent with the other attribute bindings in this class. This is a good pattern improvement that maintains the input type determination logic while using the established binding mechanism.packages/web-platform/web-elements/src/XList/XListEvents.ts (4)
90-163: LGTM! Scroll-to-upper event handler refactor is correct.The rename of
#updateEventSwitchesto_updateEventSwitchesand#handleUpperThresholdItemCountChangeto_handleUpperThresholdItemCountChangemaintains the IntersectionObserver setup and threshold monitoring logic correctly. The decorators are properly updated.
186-258: LGTM! Scroll-to-lower event handler refactor is correct.The methods
_updateScrollToLowerEventSwitchesand_handleLowerThresholdItemCountChangeare properly renamed with their IntersectionObserver logic intact. The lower threshold monitoring functionality is preserved.
355-389: LGTM! Scroll events handler refactor with correct variable reference.The rename of
#handleScrollEventsSwitchesto_handleScrollEventsSwitchesis correct, and line 366 properly useslynxscroll(matching the destructuring on line 358) to determine when to attach the throttled scroll listener. This addresses the previously identified issue.
414-433: LGTM! Edge event handlers properly refactored.The rename of
#handleScrollToUpperEdgeEventEnableto_handleScrollToUpperEdgeEventEnableand#handleScrollToLowerEdgeEventEnableto_handleScrollToLowerEdgeEventEnablemaintains the IntersectionObserver bindings for edge detection correctly.packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
48-66: The visibility change from private (#) to underscore-prefixed (_) is necessary for decorator functionality.The
@registerEventEnableStatusChangeHandlerdecorator requires public method access to register these methods on the class prototype. Private fields (#) are inaccessible to decorators under TypeScript's experimental decorator model. While now technically public, the underscore convention correctly signals these as internal methods not intended for external use.This aligns with the codebase-wide pattern mentioned in the PR objectives.
packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts (1)
34-34: LGTM! Decorator-annotated methods correctly use underscore prefix.The selective renaming from
#to_for decorator-annotated methods (lines 34, 60, 130) while keeping#for non-decorated private fields is an appropriate pattern. Decorator-annotated methods like_handleEnableConfirmEvent,_handleSendComposingInput, and_handleEnableSelectionEventnow follow a consistent convention distinguishing them from pure private fields like#domand#sendComposingInput.
…ability * enable noUnusedLocals for web-elements * add source field for supporting @rsbuild/plugin-source-build
PupilTong
force-pushed
the
p/hw/deps-support-export-source
branch
from
f578474 to
4059aa5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
115-120: Enable/disable flags for simple/complex events appear inconsistent (can break re‑enable and disposal)While unrelated to the underscore rename, the current flag handling around event wiring looks off:
#simpleRefreshViewEventsEnabledis set totruein#enableSimpleRefreshViewEvents, but never reset tofalsein#disableSimpleRefreshViewEvents. After a disable, subsequent enables will bail out early and never re‑attach thetouchendlistener.#complexRefreshViewEventEnabledis never set totruein#enableComplexRefreshViewEvents, so#disableComplexRefreshViewEvents’s guard staysfalseand none of the complex listeners (including thescrollhandler) are ever removed — even indispose().This can lead to listeners not being re‑attached when toggling, and to leaked listeners when disposing elements.
I’d strongly recommend fixing the flags while you’re in this file so enable/disable +
dispose()behave as intended. One possible adjustment:Proposed fix for event flags and cleanup
#simpleRefreshViewEventsEnabled: boolean = false; #enableSimpleRefreshViewEvents() { if (this.#simpleRefreshViewEventsEnabled) return; this.#dom.addEventListener('touchend', this.#handleTouchEndForEvent); this.#simpleRefreshViewEventsEnabled = true; } @@ #disableSimpleRefreshViewEvents() { if (this.#simpleRefreshViewEventsEnabled) { this.#dom.removeEventListener('touchend', this.#handleTouchEndForEvent); + this.#simpleRefreshViewEventsEnabled = false; } } @@ #complexRefreshViewEventEnabled: boolean = false; #enableComplexRefreshViewEvents() { if (this.#complexRefreshViewEventEnabled) return; @@ this.#dom .shadowRoot!.querySelector('#container')! .addEventListener('scroll', this.#handleScroll); + this.#complexRefreshViewEventEnabled = true; } @@ #disableComplexRefreshViewEvents() { if (this.#complexRefreshViewEventEnabled) { @@ this.#dom .shadowRoot!.querySelector('#container')! .removeEventListener('scroll', this.#handleScroll); + this.#complexRefreshViewEventEnabled = false; } }Also applies to: 147-151, 157-158, 229-246
packages/web-platform/web-elements-compat/package.json (1)
13-21: Add "source" field to exports entries to support @rsbuild/plugin-source-build.According to the plugin documentation, when a package uses the
exportsfield, thesourcefield must be added to each exports entry to point to the source code file. The pattern should be:{ "source": "./src/LinearContainer/LinearContainer.ts", "default": "./src/LinearContainer/LinearContainer.ts" }for the main entry and{ "source": "./src/LinearContainer/*", "default": "./src/LinearContainer/*" }for the wildcard entry, similar to the documented examples.packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (1)
34-40: Bind#scrollEventListenerForTransitionto preservethisand avoid private-field runtime errors
_handleEnableTransitionEventpassesthis.#scrollEventListenerForTransitiondirectly intobindSwitchToEventListener. When the DOM firesscroll, the listener will run withthisbound to the DOM element, not theXSwipeEventsinstance. Because#scrollEventListenerForTransitionuses private fields (this.#dom,this.#getContentContainer()), this will fail the private-field brand check at runtime.Everywhere else you attach private methods as listeners (e.g., in
#listeners), you use.bind(this)to fixthis. This one should follow the same pattern.Proposed fix
@registerEventEnableStatusChangeHandler('transition') - _handleEnableTransitionEvent = bindSwitchToEventListener( - this.#getContentContainer, - 'scroll', - this.#scrollEventListenerForTransition, - { passive: true }, - ); + _handleEnableTransitionEvent = bindSwitchToEventListener( + this.#getContentContainer, + 'scroll', + this.#scrollEventListenerForTransition.bind(this), + { passive: true }, + );Also applies to: 117-127
packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts (1)
31-52: Horizontal orientation check forscroll-leftis likely wrongIn
leftScrollDistanceyou gate onscrollOrientation === 'vertical', which means horizontal scrolling can be enabled even when the orientation is explicitly vertical:const leftScrollDistance = (attributeName === 'scroll-left' || attributeName === 'initial-scroll-offset') && (scrollX === '' || scrollX === 'true' || scrollOrientation === 'vertical' // ← suspicious for “left” || scrollOrientation === 'both');This looks like a typo and probably should be
'horizontal'instead of'vertical', otherwise a vertical‑onlyScrollViewcan still be scrolled horizontally wheninitial-scroll-offsetis set.Proposed fix
- const leftScrollDistance = (attributeName === 'scroll-left' - || attributeName === 'initial-scroll-offset') - && (scrollX === '' - || scrollX === 'true' - || scrollOrientation === 'vertical' - || scrollOrientation === 'both'); + const leftScrollDistance = (attributeName === 'scroll-left' + || attributeName === 'initial-scroll-offset') + && (scrollX === '' + || scrollX === 'true' + || scrollOrientation === 'horizontal' + || scrollOrientation === 'both');packages/web-platform/web-elements/src/XList/XListEvents.ts (1)
352-389: Fix boolean logic in scroll event switches (lynxscroll/lynxscrollend/snap)
#eventSwitchesstores booleans, but_handleScrollEventsSwitchesuses!== nullchecks:
this.#enableScrollEnd = lynxscrollend !== null || snap !== null;if (lynxscroll !== null || this.#enableScrollEnd) { ... }Since
false !== nullevaluates totrue, these conditions remain true regardless of whether events are enabled. This causes:
- The throttled
'scroll'listener to always attach once_handleScrollEventsSwitchesruns, even iflynxscroll/lynxscrollend/snapare disabled#enableScrollEndto be effectively alwaystrue, registering'scrollend'listeners unnecessarilyFix by treating these as booleans:
- this.#enableScrollEnd = lynxscrollend !== null || snap !== null; + this.#enableScrollEnd = lynxscrollend || snap; - if (lynxscroll !== null || this.#enableScrollEnd) { + if (lynxscroll || this.#enableScrollEnd) {This restores the intended gating: attach scroll/scrollend listeners only when at least one of
lynxscroll,lynxscrollend, orsnapis actually enabled.
🧹 Nitpick comments (4)
packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (1)
51-64: NormalizenewValueto a string once to avoid nullability issues and clarify behaviorTwo points here:
- From a typing perspective,
newValueisstring | nullbut is eventually assigned totextarea.value(string). UnderstrictNullChecksthis can be a type error. You can normalize once and work with a plain string:Proposed refactor for `_handleValue`
@registerAttributeHandler('value', false) // delay value to connectedCallback to wait the maxlength value. -_handleValue(newValue: string | null) { - if (newValue) { - const maxlength = parseFloat(this.#dom.getAttribute('maxlength') ?? ''); - if (!isNaN(maxlength)) newValue = newValue.substring(0, maxlength); - } else { - newValue = ''; - } - const textarea = this.#getTextareaElement(); - if (textarea.value !== newValue) { - textarea.value = newValue; - } -} +_handleValue(newValue: string | null) { + let nextValue = newValue ?? ''; + + if (nextValue) { + const maxlength = parseFloat(this.#dom.getAttribute('maxlength') ?? ''); + if (!isNaN(maxlength)) nextValue = nextValue.substring(0, maxlength); + } + + const textarea = this.#getTextareaElement(); + if (textarea.value !== nextValue) { + textarea.value = nextValue; + } +}
- Semantically, this now always clears the textarea (
'') when thevalueattribute is removed (newValue === null), rather than potentially leaving user‑typed content intact. That looks intentional given the comment about delaying toconnectedCallback, but worth double‑checking against XTextarea’s expected contract.packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts (1)
66-81: Guardscroll-to-indexparsing to avoid NaN → first-child fallback
_handleInitialScrollIndexusesparseFloatand passes the result directly tochildren.item(scrollValue). IfnewValis invalid (e.g."foo"),parseFloatyieldsNaNandchildren.item(NaN)effectively targets index0, which is a surprising fallback for bad input.Optional tweak for more predictable behavior:
Suggested refactor
- _handleInitialScrollIndex(newVal: string | null) { - if (newVal) { - const scrollValue = parseFloat(newVal); - const childrenElement = this.#dom.children.item(scrollValue); + _handleInitialScrollIndex(newVal: string | null) { + if (newVal) { + const index = parseInt(newVal, 10); + if (Number.isNaN(index)) return; + const childrenElement = this.#dom.children.item(index);packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts (2)
30-71: Tighten_handleSrccontrol flow to avoid noisy parse errors and ambiguous stateThe new
_handleSrcbehavior is good in spirit (error events + internal state reset), but there are a couple of rough edges:
JSON.parse(newValue || '')runs even whennewValue === null, so a normal attribute removal will always throw, get caught, and log a console error.- On parse failure you fall back to
{}and still assign it tothis.#dom[xAudioSrc], which can be ambiguous for consumers expecting a valid object withidandplay_url.Consider restructuring to short‑circuit the null and invalid cases and only update
xAudioSrcwhen the payload is valid, e.g.:Illustrative refactor
@registerAttributeHandler('src', true) - _handleSrc(newValue: string | null) { - let parsedSrc; - try { - parsedSrc = JSON.parse(newValue || '') || {}; - } catch (error) { - console.error(`JSON.parse src error: ${error}`); - parsedSrc = {}; - } - - if (newValue === null) { - this.#dom.dispatchEvent( - new CustomEvent('error', { - ...commonComponentEventSetting, - detail: { - code: XAudioErrorCode.SrcError, - msg: '', - from: 'res loader', - currentSrcID: this.#dom[xAudioSrc]?.id, - }, - }), - ); - } else if ( - parsedSrc?.id === undefined || parsedSrc?.play_url === undefined - ) { - this.#dom.dispatchEvent( - new CustomEvent('error', { - ...commonComponentEventSetting, - detail: { - code: XAudioErrorCode.SrcJsonError, - msg: '', - from: 'res loader', - currentSrcID: this.#dom[xAudioSrc]?.id, - }, - }), - ); - } - - this.#dom[xAudioSrc] = parsedSrc; - this.#dom[xAudioBlob] = undefined; - this.#dom.stop(); - } + _handleSrc(newValue: string | null) { + if (newValue === null) { + // src removed: emit error + clear state and stop + this.#dom.dispatchEvent( + new CustomEvent('error', { + ...commonComponentEventSetting, + detail: { + code: XAudioErrorCode.SrcError, + msg: '', + from: 'res loader', + currentSrcID: this.#dom[xAudioSrc]?.id, + }, + }), + ); + this.#dom[xAudioSrc] = undefined; + this.#dom[xAudioBlob] = undefined; + this.#dom.stop(); + return; + } + + let parsedSrc; + try { + parsedSrc = JSON.parse(newValue); + } catch (error) { + console.error(`JSON.parse src error: ${error}`); + this.#dom.dispatchEvent( + new CustomEvent('error', { + ...commonComponentEventSetting, + detail: { + code: XAudioErrorCode.SrcJsonError, + msg: '', + from: 'res loader', + currentSrcID: this.#dom[xAudioSrc]?.id, + }, + }), + ); + return; + } + + if (parsedSrc?.id === undefined || parsedSrc?.play_url === undefined) { + this.#dom.dispatchEvent( + new CustomEvent('error', { + ...commonComponentEventSetting, + detail: { + code: XAudioErrorCode.SrcJsonError, + msg: '', + from: 'res loader', + currentSrcID: this.#dom[xAudioSrc]?.id, + }, + }), + ); + return; + } + + this.#dom[xAudioSrc] = parsedSrc; + this.#dom[xAudioBlob] = undefined; + this.#dom.stop(); + }Also, from this file alone it isn’t obvious where
xAudioSrcultimately drives the<audio>element’ssrc(the old#setAudioSrc = bindToAttribute(..., 'src')binding is gone). If that wiring has moved intoXAudioTT/utils, all good; otherwise this refactor might inadvertently stop updating the real audio element.
82-96: Ensurevisibilitychangelistener is cleaned up on element disposal
_handlePauseOnHideadds adocument‑levelvisibilitychangelistener when the attribute is present and removes it only when the attribute is removed. If an element withpause-on-hidestays truthy and is then detached from the DOM, the listener will still hold a reference to this instance and keep callingthis.#dom.pause().To avoid leaks and unexpected behavior, consider unregistering the listener in a lifecycle hook (
dispose/disconnectedCallback) as a safety net, even when the attribute stays set.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
.changeset/heavy-birds-beg.md(1 hunks)packages/web-platform/web-elements-compat/package.json(1 hunks)packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts(1 hunks)packages/web-platform/web-elements-reactive/package.json(1 hunks)packages/web-platform/web-elements/elements.css(1 hunks)packages/web-platform/web-elements/index.css(1 hunks)packages/web-platform/web-elements/package.json(1 hunks)packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollAttributes.ts(2 hunks)packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts(4 hunks)packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts(3 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts(0 hunks)packages/web-platform/web-elements/src/XImage/DropShadow.ts(1 hunks)packages/web-platform/web-elements/src/XImage/ImageEvents.ts(2 hunks)packages/web-platform/web-elements/src/XImage/ImageSrc.ts(2 hunks)packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts(5 hunks)packages/web-platform/web-elements/src/XInput/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XInput/XInputAttribute.ts(2 hunks)packages/web-platform/web-elements/src/XInput/XInputEvents.ts(3 hunks)packages/web-platform/web-elements/src/XList/ListItemAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XList/XListEvents.ts(8 hunks)packages/web-platform/web-elements/src/XList/XListWaterfall.ts(2 hunks)packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts(2 hunks)packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts(2 hunks)packages/web-platform/web-elements/src/XSvg/XSvg.ts(3 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts(2 hunks)packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts(1 hunks)packages/web-platform/web-elements/src/XText/InlineImage.ts(1 hunks)packages/web-platform/web-elements/src/XText/RawText.ts(1 hunks)packages/web-platform/web-elements/src/XText/XTextTruncation.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/Placeholder.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts(3 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts(1 hunks)packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts(3 hunks)packages/web-platform/web-elements/src/XView/BlurRadius.ts(1 hunks)packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts(1 hunks)packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts(1 hunks)packages/web-platform/web-elements/tsconfig.json(0 hunks)packages/web-platform/web-worker-rpc/package.json(1 hunks)packages/web-platform/web-worker-rpc/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewSlotNgTouchEventsHandler.ts
- packages/web-platform/web-elements/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (20)
- .changeset/heavy-birds-beg.md
- packages/web-platform/web-elements/src/XTextarea/Placeholder.ts
- packages/web-platform/web-worker-rpc/src/index.ts
- packages/web-platform/web-elements/src/XText/XTextTruncation.ts
- packages/web-platform/web-elements-reactive/package.json
- packages/web-platform/web-elements/src/XSwiper/XSwiperAutoScroll.ts
- packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts
- packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts
- packages/web-platform/web-elements/src/XList/ListItemAttributes.ts
- packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts
- packages/web-platform/web-elements/src/XImage/DropShadow.ts
- packages/web-platform/web-elements/src/XInput/XInputEvents.ts
- packages/web-platform/web-elements/src/XInput/XInputAttribute.ts
- packages/web-platform/web-elements/src/XSvg/XSvg.ts
- packages/web-platform/web-elements/src/XInput/Placeholder.ts
- packages/web-platform/web-elements/package.json
- packages/web-platform/web-elements/src/XImage/ImageSrc.ts
- packages/web-platform/web-elements/src/XTextarea/XTextareaEvents.ts
- packages/web-platform/web-elements/src/XImage/ImageEvents.ts
- packages/web-platform/web-elements/src/XTextarea/TextareaBaseAttributes.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
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.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.tspackages/web-platform/web-elements/index.csspackages/web-platform/web-elements/elements.csspackages/web-platform/web-elements/src/XList/XListAttributes.ts
📚 Learning: 2025-07-16T06:28:26.463Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 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-elements/index.csspackages/web-platform/web-elements/elements.css
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XText/InlineImage.tspackages/web-platform/web-elements/src/XList/XListWaterfall.tspackages/web-platform/web-elements/src/XText/RawText.tspackages/web-platform/web-elements/src/XList/XListAttributes.tspackages/web-platform/web-elements/src/ScrollView/ScrollAttributes.tspackages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.tspackages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.tspackages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.tspackages/web-platform/web-elements/src/XView/BlurRadius.tspackages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.tspackages/web-platform/web-elements/src/XList/XListEvents.tspackages/web-platform/web-elements/src/XInput/InputBaseAttributes.tspackages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts
📚 Learning: 2025-07-16T06:25:41.055Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts:214-217
Timestamp: 2025-07-16T06:25:41.055Z
Learning: In the lynx-stack codebase, CSS strings produced by `genCssContent` are considered trusted developer input, so additional sanitization/escaping is not required.
Applied to files:
packages/web-platform/web-elements/elements.css
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-worker-rpc/package.json
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-worker-rpc/package.json
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.tspackages/web-platform/web-elements/src/XList/XListEvents.ts
📚 Learning: 2025-11-11T08:05:14.163Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.163Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts
🧬 Code graph analysis (5)
packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToStyle(8-8)registerAttributeHandler(20-20)packages/web-platform/web-elements-reactive/src/registerAttributeHandler.ts (1)
registerAttributeHandler(19-30)
packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts (3)
packages/web-platform/web-elements-reactive/src/index.ts (3)
bindToStyle(8-8)registerAttributeHandler(20-20)registerEventEnableStatusChangeHandler(24-24)packages/web-platform/web-elements-reactive/src/registerAttributeHandler.ts (1)
registerAttributeHandler(19-30)packages/web-platform/web-elements-reactive/src/registerEventStatusChangedHandler.ts (1)
registerEventEnableStatusChangeHandler(16-24)
packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (2)
bindToAttribute(6-6)bindToStyle(8-8)
packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (2)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindSwitchToEventListener(5-5)packages/web-platform/web-elements-reactive/src/bindSwitchToEventListener.ts (1)
bindSwitchToEventListener(8-32)
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
packages/web-platform/web-elements-reactive/src/index.ts (1)
bindToAttribute(6-6)
🔇 Additional comments (24)
packages/web-platform/web-elements/src/XText/InlineImage.ts (1)
24-28: Handler rename to_handleSrcpreserves behavior and matches new conventionThe refactor keeps the
srchandling logic identical while aligning the handler name with the underscore “private-like” pattern used elsewhere; no functional or typing concerns here.packages/web-platform/web-worker-rpc/package.json (1)
14-20: LGTM! Exports field correctly configured.The addition of the
exportsfield withtypes,default, andsourceentries aligns with the PR objective to support @rsbuild/plugin-source-build. The structure follows Node.js package export conventions, and backward compatibility is maintained with the existingmainandtypingsfields.packages/web-platform/web-elements/index.css (1)
5-6: Consolidated CSS import looks good; confirm cascade stays the sameSwitching from multiple component-level imports to a single
./elements.cssimport simplifies the entrypoint. Just ensureelements.csspreserves the previous component import order so CSS cascade and specificity behavior don’t change unexpectedly.packages/web-platform/web-elements/elements.css (1)
1-18: elements.css aggregation file is well-structured and paths look consistentThe new header and explicit
@importlist for each component stylesheet provide a clear, centralized aggregation point for web-elements styles. The relative./src/...paths are consistent and should maintain existing behavior when consumed via the new export.packages/web-platform/web-elements/src/XView/BlurRadius.ts (1)
23-32: Handler rename preserves behavior and fits refactor goalThe rename to
_handleBlurRadiuskeeps the attribute handling logic and styling behavior identical while aligning with the new underscore convention for handler methods. No issues spotted with the blur CSS generation or dynamic style updates.packages/web-platform/web-elements/src/XSwiper/XSwiperCircular.ts (1)
161-161: LGTM! Decorator-compatible naming convention.The refactor from
#(JavaScript private fields) to_(underscore-prefixed) makes these methods accessible to the@registerAttributeHandlerdecorator, which is necessary since true private fields cannot be accessed by decorators. The rest of the class methods retain the#prefix, creating a clear pattern where only decorator-registered handlers use the underscore convention.Also applies to: 195-195
packages/web-platform/web-elements/src/XText/RawText.ts (1)
19-26: The visibility change from#handleTextto_handleTextrequires verification of the decorator implementation.Modern JavaScript decorators can be applied to private methods, so the original claim about decorator compatibility may not be accurate. Verify that the
@registerAttributeHandlerdecorator actually requires public accessibility (e.g., whether it wraps or invokes the method at class definition time). If the method doesn't need to be called from outside the class definition, consider whether a true private method could still work.packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts (1)
48-66: Renaming from#private methods to underscored methods changes runtime visibilityThese handlers used to be true private fields/methods (
#handleComplexEventEnableAttributes,#handleXEnableHeaderOffsetEvent); now they are normal methods_handleComplexEventEnableAttributes/_handleXEnableHeaderOffsetEvent. That keeps the decorator wiring intact but does loosen encapsulation (instances are now able to call these methods from userland JS).If this class is only ever used internally and instances are not exposed, this is probably fine and a reasonable tradeoff for
noUnusedLocals. Otherwise, please double‑check that there’s no expectation elsewhere that these stay hard‑private, or consider leaving a brief comment noting they’re “logically private” but intentionally not#/privatedue to TS/decorator constraints.Also applies to: 68-93
packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts (1)
8-8: LGTM! Type-only import is correct here.The change to a type-only import is appropriate since
WebComponentClassis only used for type annotations (line 127) and type assertions (line 118), which are erased at compile time. This aligns with the PR objective to enablenoUnusedLocals.packages/web-platform/web-elements-compat/package.json (1)
18-21: Verify that the wildcard export is intentional and necessary.The new
"./LinearContainer/*"wildcard export exposes all files in thesrc/LinearContainer/directory. Confirm that all files in this directory are intended to be publicly accessible and this exposure is necessary. If only specific files should be exposed, consider using explicit exports instead.packages/web-platform/web-elements/src/XTextarea/XTextareaAttributes.ts (2)
32-36: Confirm‑enter handler rename looks fineThe rename to
_handleConfirmEnterkeeps the boolean‑presence semantics (#confirmEnter = newVal !== null) and remains wired via@registerAttributeHandler('confirm-enter', true), so behavior is preserved while aligning with the new underscore convention.
38-49: Attribute/style handler refactors preserve existing behaviorThe
_handleDisabled,_handleMaxHeight, and_handleMinHeightbindings still delegate tobindToAttribute/bindToStylewith the same arguments, so the only change is naming. This keeps the existing DOM behavior intact while matching the updated handler naming pattern.packages/web-platform/web-elements/src/XSwiper/XSwiperEvents.ts (1)
169-186: Underscore rename for_enableScrollEventProcessorlooks correct and behavior‑preservingThe rename to
_enableScrollEventProcessorkeeps the method signature and body intact, and the event-name narrowing on#eventSwitchesplus the aggregatechangeEventEnabledcomputation remain unchanged. Decorator usage is unaffected, so this is a safe, style-only change aligned with the PR’s noUnusedLocals/“private-like” naming pattern.packages/web-platform/web-elements/src/XSwiper/XSwiperIndicator.ts (1)
41-75: Refactor from private (#) to conventionally private (_) fields for decorator compatibility.These five handler fields are renamed from hash-prefixed (truly private) to underscore-prefixed (conventionally private) to support the
@registerAttributeHandlerdecorator andbindToStyle()reactive system. This visibility change exposes implementation details outside the class, but the pattern is limited to these specific handlers—other private fields like#domand#numOfChildElementremain hash-prefixed. Consider adding a code comment explaining why these fields use underscore-prefix to help future maintainers understand the pattern.packages/web-platform/web-elements/src/XFoldViewNg/XFoldviewNgEvents.ts (1)
25-40: Granularity and offset event handler renames look correctThe switch from
#handleGranularity/#enableOffsetEventto_handleGranularity/_enableOffsetEventpreserves the decorators, event wiring, and scroll offset dispatch logic; no behavioral changes introduced here.packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts (1)
31-67: Mechanical rename of overlay handlers; logic preservedThe rename to
_handleEventsPassThroughand_handleVisiblekeeps the same decorators and bodies, so events-pass-through wiring and dialog visibility behavior (includingshowoverlay/dismissoverlayevents) remain unchanged.packages/web-platform/web-elements/src/ScrollView/ScrollViewEvents.ts (1)
68-74: Scroll event and threshold handler renames keep behavior intactThe renamed handlers (
_handleScrollUpperThresholdEventEnabled,_handleScrollLowerThresholdEventEnabled,_updateUpperThreshold,_updateLowerThreshold,_handleScrollEventEnabled,_handleScrollEndEventEnabled) still:
- Use the same decorators for
scrolltoupper,scrolltolower,lynxscroll, andlynxscrollend.- Toggle the same enabling attributes and intersection observers.
- Bind
upper-threshold/lower-thresholdtoflex-basisviabindToStylewith the same transform.This refactor looks behaviorally neutral and consistent with the rest of the PR.
Also applies to: 82-88, 96-108, 160-170
packages/web-platform/web-elements/src/XInput/InputBaseAttributes.ts (1)
33-99: Well-structured refactoring that enables decorator access.The migration from
#(JavaScript private fields) to_(underscore-prefix convention) for decorated attribute handlers is necessary because TypeScript decorators cannot access truly private#fields. The distinction is well-maintained:
_prefix: decorated members accessible to the attribute handler system#prefix: truly private helpers like#setTypeand#setInputmodeThe introduction of dedicated bindings (
#setType,#setInputmode) at lines 59-60 and their usage in_handleTypeimproves code clarity. The logic correctly mapsInputTypevalues to appropriateinputModeandinputTypesettings.packages/web-platform/web-elements/src/XList/XListAttributes.ts (1)
26-39: Sticky offset & span-count handler rename is safeThe switch from
#handlerStickyOffset/#handlerCountto_handlerStickyOffset/_handlerCountkeeps the samebindToStylewiring and parsing behavior, so attribute handling and styling should remain unchanged while aligning with the new soft‑private naming convention.packages/web-platform/web-elements/src/XList/XListWaterfall.ts (2)
32-75: scrolltolower enable handler rename preserves behaviorRenaming
#handleXEnableHeaderOffsetEventto_handleXEnableHeaderOffsetEventunder@registerEventEnableStatusChangeHandler('scrolltolower')keeps the same DOM/query logic and Firefox workarounds, so event enabling and threshold marker positioning behavior remain intact.
209-244: list-type attribute handler rename is consistent and non-breaking
#handlerListType→_handlerListTypekeeps the same waterfall container creation, observer setup/teardown, and slot cleanup logic. The decorator still targets the correct method, so switchinglist-typeto/from'waterfall'should behave as before.packages/web-platform/web-elements/src/XList/XListEvents.ts (3)
89-163: Upper threshold event switch and attribute handler rename look correct
#updateEventSwitches→_updateEventSwitchesand#handleUpperThresholdItemCountChange→_handleUpperThresholdItemCountChangekeep the same IntersectionObserver setup, mutation observer wiring, and DOM selection logic forupper-threshold-item-count. The event/attribute decorators still bind to the right instance members, so scroll-to-upper behavior should be unchanged.
185-258: Lower threshold event switch and attribute handler rename are safeThe renames to
_updateScrollToLowerEventSwitchesand_handleLowerThresholdItemCountChangepreserve the previous logic for configuring the lower IntersectionObserver and reacting tolower-threshold-item-countchanges.#childrenObserverreuse semantics and observer reconnection on children changes remain the same.
413-433: Edge scroll event enable handlers rename cleanlyRenaming
#handleScrollToUpperEdgeEventEnable/#handleScrollToLowerEdgeEventEnableto_handleScrollToUpperEdgeEventEnable/_handleScrollToLowerEdgeEventEnablewhile keeping the same attribute toggling andbindToIntersectionObserverusage retains prior behavior forscrolltoupperedge/scrolltoloweredgeevents.
There was a problem hiding this comment.
Pull request overview
This PR refactors the web-elements codebase to improve code quality and maintainability by enabling stricter TypeScript checking, cleaning up unused code, fixing existing bugs, and adding build tooling support.
- Enable
noUnusedLocalsTypeScript compiler option and remove unused imports/variables - Add
sourcefield mappings to package.json exports for @rsbuild/plugin-source-build support - Fix bug in XViewpagerNg where offsetchange event incorrectly modified the change enable flag
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-platform/web-worker-rpc/src/index.ts | Exports additional async RPC endpoint types |
| packages/web-platform/web-worker-rpc/package.json | Adds exports field with source mapping for build plugin support |
| packages/web-platform/web-elements/tsconfig.json | Removes noUnusedLocals: false to enable strict unused variable checking |
| packages/web-platform/web-elements/src/common/CommonEventsAndMethods.ts | Adds dispose() method and ensures proper cleanup of resize observer |
| packages/web-platform/web-elements/src/XViewpagerNg/XViewpagerNgEvents.ts | Fixes bug where offsetchange handler incorrectly set enableChange instead of enableOffsetChange; changes method visibility |
| packages/web-platform/web-elements/src/XView/BlurRadius.ts | Changes method visibility from private to protected |
| packages/web-platform/web-elements/src/XTextarea/*.ts | Changes method visibility from private to protected for attribute and event handlers |
| packages/web-platform/web-elements/src/XText/*.ts | Changes method visibility from private to protected for methods called in connectedCallback |
| packages/web-platform/web-elements/src/XSwiper/*.ts | Changes method visibility from private to protected for attribute and event handlers |
| packages/web-platform/web-elements/src/XSvg/XSvg.ts | Removes unused bindToStyle import |
| packages/web-platform/web-elements/src/XRefreshView/XRefreshViewEventsEmitter.ts | Removes unused scrollHeight variable |
| packages/web-platform/web-elements/src/XOverlayNg/XOverlayAttributes.ts | Changes method visibility from private to protected |
| packages/web-platform/web-elements/src/XList/*.ts | Changes method visibility and fixes bug where scroll variable incorrectly referenced instead of lynxscroll |
| packages/web-platform/web-elements/src/XInput/*.ts | Changes method visibility from private to protected and removes unused #inputType field |
| packages/web-platform/web-elements/src/XImage/*.ts | Changes method visibility from private to protected for methods called in connectedCallback |
| packages/web-platform/web-elements/src/XFoldViewNg/*.ts | Removes unused isChromium import and changes method visibility |
| packages/web-platform/web-elements/src/XAudioTT/XAudioAttribute.ts | Removes unused #setAudioSrc field and changes method visibility |
| packages/web-platform/web-elements/src/ScrollView/*.ts | Changes method visibility from private to protected and adds minor formatting |
| packages/web-platform/web-elements/package.json | Adds source field mappings to all component exports and adds elements.css export |
| packages/web-platform/web-elements/index.css | Consolidates CSS imports by delegating to elements.css |
| packages/web-platform/web-elements/elements.css | New file containing all element-specific CSS imports |
| packages/web-platform/web-elements-reactive/package.json | Adds exports field with source mapping |
| packages/web-platform/web-elements-compat/src/LinearContainer/LinearContainer.ts | Changes WebComponentClass to type-only import |
| packages/web-platform/web-elements-compat/package.json | Adds wildcard export for LinearContainer subdirectories |
| .changeset/heavy-birds-beg.md | Documents the refactoring changes as a patch release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/web-platform/web-elements/src/ScrollView/FadeEdgeLengthAttribute.ts
Show resolved
Hide resolved
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/lynx-bundle-rslib-config@0.1.0 ### Minor Changes - Update external bundle minimum SDK version to 3.5. ([#2037](#2037)) ### Patch Changes - Fix `globDynamicComponentEntry is not defined` error when minify is enabled in external bundle consumer. ([#2058](#2058)) ## @lynx-js/web-elements@0.10.0 ### Minor Changes - chore: migrate all @lynx-js/web-elements-\* packages into one ([#2057](#2057)) ### Before ```js import "@lynx-js/web-elements-template"; import "@lynx-js/web-elements-compat/LinearContainer"; ``` ### After ```js import "@lynx-js/web-elements/html-templates"; import "@lynx-js/web-elements/compat/LinearContainer"; ``` ### Patch Changes - refactor: change code structure for improved readability and maintainability ([#2004](#2004)) - enable noUnusedLocals for web-elements - add source field for supporting @rsbuild/plugin-source-build This is a part of #1937 ## @lynx-js/react@0.115.2 ### Patch Changes - Fix `undefined factory (react:background)/./node_modules/.pnpm/@lynx-js+react...` error when loading a standalone lazy bundle after hydration. ([#2048](#2048)) - Partially fix "main-thread.js exception: TypeError: cannot read property '\_\_elements' of undefined" by recursively calling `snapshotDestroyList`. ([#2041](#2041)) - Fix a bug where React throws `CtxNotFound` error when lazy bundle resolves after unmount. ([#2003](#2003)) ## @lynx-js/rspeedy@0.12.4 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.19.3 ## @lynx-js/external-bundle-rsbuild-plugin@0.0.1 ### Patch Changes - Introduce `@lynx-js/external-bundle-rsbuild-plugin`. ([#2006](#2006)) ```ts // lynx.config.ts import { pluginExternalBundle } from "@lynx-js/external-bundle-rsbuild-plugin"; import { pluginReactLynx } from "@lynx-js/react-rsbuild-plugin"; export default { plugins: [ pluginReactLynx(), pluginExternalBundle({ externals: { lodash: { url: "http://lodash.lynx.bundle", background: { sectionPath: "background" }, mainThread: { sectionPath: "mainThread" }, }, }, }), ], }; ``` - Updated dependencies \[[`491c5ef`](491c5ef)]: - @lynx-js/externals-loading-webpack-plugin@0.0.2 ## @lynx-js/react-rsbuild-plugin@0.12.3 ### Patch Changes - expose LAYERS via `api.expose` for other rsbuild plugins. ([#2006](#2006)) - Updated dependencies \[[`cd89bf9`](cd89bf9)]: - @lynx-js/template-webpack-plugin@0.10.1 - @lynx-js/react-alias-rsbuild-plugin@0.12.3 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.4 - @lynx-js/react-webpack-plugin@0.7.3 - @lynx-js/css-extract-webpack-plugin@0.7.0 ## @lynx-js/web-constants@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 ## @lynx-js/web-core@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 - @lynx-js/web-constants@0.19.3 - @lynx-js/web-worker-runtime@0.19.3 - @lynx-js/web-mainthread-apis@0.19.3 ## @lynx-js/web-mainthread-apis@0.19.3 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.19.3 ## @lynx-js/web-worker-rpc@0.19.3 ### Patch Changes - feat: support lazy message port assigning in web-worker-rpc ([#2040](#2040)) ## @lynx-js/web-worker-runtime@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 - @lynx-js/web-constants@0.19.3 - @lynx-js/web-mainthread-apis@0.19.3 ## @lynx-js/externals-loading-webpack-plugin@0.0.2 ### Patch Changes - Export `ExternalValue` ts type. ([#2037](#2037)) ## @lynx-js/template-webpack-plugin@0.10.1 ### Patch Changes - fix: pass updated css from encodeData to resolvedEncodeOptions ([#2053](#2053)) Previously, the initial CSS was used in resolvedEncodeOptions instead of the potentially updated CSS from encodeData after the beforeEncode hook. This fix ensures resolvedEncodeOptions receives the latest CSS data. ## create-rspeedy@0.12.4 ## @lynx-js/react-alias-rsbuild-plugin@0.12.3 ## upgrade-rspeedy@0.12.4 ## @lynx-js/web-core-server@0.19.3 ## @lynx-js/web-rsbuild-server-middleware@0.19.3 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This is a part of #1937
Summary by CodeRabbit
New Features
Bug Fixes
Chores / Refactor
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist