-
Notifications
You must be signed in to change notification settings - Fork 111
feat: support elementTemplate for web #1374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 33cef98 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 |
📝 WalkthroughWalkthroughA patch update introduces support for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
100-100: Consider type safety and property access consistency.The elementTemplate property addition looks correct, but there are a few considerations:
- Property access inconsistency: This uses bracket notation while other properties use dot notation (e.g.,
encodeOptions.manifeston line 91).- Potential undefined value: No validation that
elementTemplateexists inencodeOptions.Consider using consistent dot notation and potentially providing a default value:
- elementTemplate: encodeOptions['elementTemplate'], + elementTemplate: encodeOptions.elementTemplate ?? {},Alternatively, if bracket notation is intentional due to typing constraints, consider adding a comment explaining why.
packages/webpack/template-webpack-plugin/test/cases/web/element-template/b.js (1)
6-13: LGTM! Test correctly verifies elementTemplate presence.The test successfully validates that the elementTemplate property is included in the generated JSON. The async file reading and JSON parsing logic is correct.
Consider making the test more specific about what constitutes a valid elementTemplate:
const { elementTemplate } = JSON.parse(fileContent); - expect(elementTemplate).toBeTruthy(); + expect(elementTemplate).toBeDefined(); + expect(typeof elementTemplate).toBe('object');This would ensure elementTemplate is not just truthy but is actually an object as expected.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/afraid-crews-dig.md(1 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)packages/webpack/template-webpack-plugin/test/cases/web/element-template/a.js(1 hunks)packages/webpack/template-webpack-plugin/test/cases/web/element-template/b.js(1 hunks)packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.js(1 hunks)packages/webpack/template-webpack-plugin/test/cases/web/element-template/test.config.cjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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: colinaaa
PR: lynx-family/lynx-stack#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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
.changeset/afraid-crews-dig.md (2)
Learnt from: colinaaa
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: colinaaa
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.
packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.js (3)
Learnt from: colinaaa
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.
Learnt from: PupilTong
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.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
🧬 Code Graph Analysis (1)
packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.js (1)
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
WebEncodePlugin(14-123)
🔇 Additional comments (9)
packages/webpack/template-webpack-plugin/test/cases/web/element-template/a.js (1)
1-1: LGTM! Simple test fixture correctly implemented.This test entry point serves its purpose well as part of the elementTemplate feature test suite.
.changeset/afraid-crews-dig.md (1)
1-5: LGTM! Changeset correctly formatted for the feature addition.The patch version update is appropriate for adding elementTemplate support, and the description aligns with the PR objectives.
packages/webpack/template-webpack-plugin/test/cases/web/element-template/test.config.cjs (1)
1-6: LGTM! Test configuration correctly structured.The configuration properly specifies the bundle path for the elementTemplate test case and uses appropriate JSDoc typing.
packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.js (6)
1-14: LGTM! Configuration follows established patterns.The import statement and entry configuration are consistent with other test cases in the repository. The main-thread entries with
.lepus.jsextension and the organized output structure align with the lynx-stack architecture.
16-18: LGTM! WebEncodePlugin configuration is correct.The plugin is properly configured to include only the 'a.js' module, which aligns with the test case requirements for elementTemplate support.
19-24: LGTM! LynxTemplatePlugin configuration follows conventions.The plugin configuration correctly targets the 'a' and 'a:main-thread' chunks with appropriate filename and intermediate directory settings, consistent with other test cases.
26-36: LGTM! Asset processing follows established pattern.The logic for marking assets with the 'lynx:main-thread' flag is identical to other test configurations and correctly implements the required asset processing.
54-57: LGTM! File structure is complete and syntactically correct.The configuration properly closes the plugins array and maintains valid JavaScript syntax throughout.
37-53: ElementTemplate injection test coverage confirmed
- The hook in
packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.jsinjectsencodeData.elementTemplate = {}as intended.- The Vitest in the same folder reads
.rspeedy/a/tasm.jsonand asserts:it('should have elementTemplate', async () => { const { elementTemplate } = JSON.parse(/* file contents */); expect(elementTemplate).toBeTruthy(); });- No further changes required.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
packages/webpack/template-webpack-plugin/test/cases/web/element-template/rspack.config.js
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #1374 will not alter performanceComparing Summary
|
Web Explorer#3434 Bundle Size — 352.53KiB (0%).33cef98(current) vs 92445ad main#3426(baseline) Bundle metrics
Bundle size by type
|
| Current #3434 |
Baseline #3426 |
|
|---|---|---|
226.4KiB |
226.4KiB |
|
94.29KiB |
94.29KiB |
|
31.83KiB |
31.83KiB |
Bundle analysis report Branch PupilTong:p/hw/element-template Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#3444 Bundle Size — 235.18KiB (0%).33cef98(current) vs 92445ad main#3436(baseline) Bundle metrics
|
| Current #3444 |
Baseline #3436 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
156 |
156 |
|
63 |
63 |
|
45.94% |
45.94% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #3444 |
Baseline #3436 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
89.42KiB |
89.42KiB |
Bundle analysis report Branch PupilTong:p/hw/element-template Project dashboard
Generated by RelativeCI Documentation Report issue
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Minor Changes - Add support for Lynx UI plugin system with configurable options. ([#1363](#1363)) - Introduced `lynxUIPlugins` option in `createLynxPreset`, allowing userland opt-in to Lynx UI specific plugins. - Implemented `uiVariants` plugin as the first UI plugin, supporting `ui-*` variant prefixes (e.g. `ui-checked`, `ui-open`) with customizable mappings. ## @lynx-js/[email protected] ### Patch Changes - Fix crash caused by not removing event listeners during destroy. ([#1379](#1379)) - Fix missing "type" in "update-list-info" in hydrate ([#1392](#1392)) ## @lynx-js/[email protected] ### Patch Changes - Bump Rsbuild v1.4.12 with Rspack v1.4.11. ([#1326](#1326)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`e9edca0`](e9edca0), [`6f37db2`](6f37db2)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - fix: crash on chrome<96 ([#1361](#1361)) <wasm-bindgen/wasm-bindgen#4211 (comment)> <WebAssembly/binaryen#7358> The rust toolchain enables WASM feature `reference types` by default. However this feature is not supported by chromium lower than version 96 Therefore we found a workaround for it. In this implementation we detect if browser supports `reference types` first. If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off. - Updated dependencies \[[`22ca433`](22ca433), [`8645d12`](8645d12), [`143e481`](143e481)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: bundle web-core-server ([#819](#819)) ## @lynx-js/[email protected] ### Patch Changes - feat: support color style for x-textarea ([#1382](#1382)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - fix: crash on chrome<96 ([#1361](#1361)) <wasm-bindgen/wasm-bindgen#4211 (comment)> <WebAssembly/binaryen#7358> The rust toolchain enables WASM feature `reference types` by default. However this feature is not supported by chromium lower than version 96 Therefore we found a workaround for it. In this implementation we detect if browser supports `reference types` first. If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off. - Updated dependencies \[[`22ca433`](22ca433)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - optimize IO for all-on-ui: make startMainThreadWorker async and defer import ([#1385](#1385)) - Updated dependencies \[[`22ca433`](22ca433), [`143e481`](143e481)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support Rspack v1.4.9. ([#1351](#1351)) ## @lynx-js/[email protected] ### Patch Changes - feat: support elementTemplate for web ([#1374](#1374)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
Checklist
#1042