feat: Support frame element on web#2604
Conversation
🦋 Changeset detectedLatest commit: c3fa234 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
📝 WalkthroughWalkthroughThis PR adds comprehensive Lynx frame element support to the web platform. It establishes ChangesFrame Element API Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
PupilTong
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
PupilTong
force-pushed
the
codex/web-core-frame-element
branch
from
b6f1897 to
ea6461e
Compare
PupilTong
force-pushed
the
codex/web-core-frame-element
branch
from
ea6461e to
c3fa234
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts (1)
305-327: 💤 Low valueConsider more explicit assertions for auto-sizing tests.
The current tests verify that
auto-height='true'andheight != 'auto', butnot.toHaveAttributepasses in multiple scenarios (attribute missing, or attribute present with different value). While this may be intentional, more explicit assertions would make the test expectations clearer.📝 More explicit alternative
test('api-frame-auto-height', async ({ page }, { title }) => { await goto(page, title); await expect(page.locator('#target')).toHaveAttribute( 'auto-height', 'true', ); - await expect(page.locator('#target')).not.toHaveAttribute( - 'height', - 'auto', - ); + // More explicit: verify height attribute is absent entirely, or has explicit pixel value + const heightAttr = await page.locator('#target').getAttribute('height'); + expect(heightAttr).not.toBe('auto'); });Or use a comment to clarify intent:
+ // Verify that explicit height dimension is not set to 'auto' when auto-height is enabled await expect(page.locator('#target')).not.toHaveAttribute( 'height', 'auto', );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts` around lines 305 - 327, The auto-sizing tests (tests 'api-frame-auto-height' and 'api-frame-auto-width' using page.locator('#target')) currently use not.toHaveAttribute which is ambiguous; change these to explicit checks: either assert the target does not have the attribute (expect(await locator.getAttribute('height')).toBeNull()) or read the attribute value and assert it's not the string 'auto' (const h = await locator.getAttribute('height'); expect(h).not.toBe('auto')). Apply the same explicit pattern for 'width' in the 'api-frame-auto-width' test so the intent is unambiguous.packages/web-platform/web-core-e2e/tests/reactlynx/api-frame-bindload/index.jsx (1)
7-11: 💤 Low valueConsider a more semantic initial state for statusCode.
The initial
statusCode: -1works as a sentinel value, but usingnullor a separate loading state would be more explicit and avoid potential confusion with HTTP status codes.💡 Alternative approach
- const [detail, setDetail] = useState({ - statusCode: -1, - statusMessage: '', - url: '', - }); + const [detail, setDetail] = useState({ + statusCode: null, + statusMessage: '', + url: '', + });Or use a more explicit loading state:
+ const [isLoading, setIsLoading] = useState(true); const [detail, setDetail] = useState({ - statusCode: -1, + statusCode: 0, statusMessage: '', url: '', });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web-platform/web-core-e2e/tests/reactlynx/api-frame-bindload/index.jsx` around lines 7 - 11, The initial state for detail uses a sentinel -1 for statusCode; change this to a more semantic value by using null for statusCode or add an explicit loading flag (e.g., add isLoading to the state) so callers can distinguish "not loaded" from valid HTTP codes; update the useState initializer for detail and any code branches that read detail.statusCode (and any UI that checks it) to handle null or consult the new isLoading flag instead of comparing against -1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/web-platform/web-core-e2e/tests/reactlynx.spec.ts`:
- Around line 305-327: The auto-sizing tests (tests 'api-frame-auto-height' and
'api-frame-auto-width' using page.locator('#target')) currently use
not.toHaveAttribute which is ambiguous; change these to explicit checks: either
assert the target does not have the attribute (expect(await
locator.getAttribute('height')).toBeNull()) or read the attribute value and
assert it's not the string 'auto' (const h = await
locator.getAttribute('height'); expect(h).not.toBe('auto')). Apply the same
explicit pattern for 'width' in the 'api-frame-auto-width' test so the intent is
unambiguous.
In
`@packages/web-platform/web-core-e2e/tests/reactlynx/api-frame-bindload/index.jsx`:
- Around line 7-11: The initial state for detail uses a sentinel -1 for
statusCode; change this to a more semantic value by using null for statusCode or
add an explicit loading flag (e.g., add isLoading to the state) so callers can
distinguish "not loaded" from valid HTTP codes; update the useState initializer
for detail and any code branches that read detail.statusCode (and any UI that
checks it) to handle null or consult the new isLoading flag instead of comparing
against -1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 972511bf-e35c-4c65-99da-f15d1cb9cf22
📒 Files selected for processing (18)
.changeset/frame-web-core.mdpackages/web-platform/web-core-e2e/tests/reactlynx.spec.tspackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-auto-height/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-auto-width/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-bindload/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-data-update/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-data/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-element-map/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-global-props/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-inner/index.jsxpackages/web-platform/web-core-e2e/tests/reactlynx/api-frame-src/index.jsxpackages/web-platform/web-core/src/constants.rspackages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rspackages/web-platform/web-core/ts/client/mainthread/LynxView.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/constants.tspackages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/types/IElementPAPI.ts
Merging this PR will degrade performance by 5.37%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 002-hello-reactLynx-destroyBackground |
863.5 µs | 912.4 µs | -5.37% |
| ⚡ | transform 1000 view elements |
43.1 ms | 40 ms | +7.68% |
Comparing PupilTong:codex/web-core-frame-element (c3fa234) with main (33b124f)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Web Explorer#9621 Bundle Size — 901.01KiB (+0.11%).c3fa234(current) vs 33b124f main#9602(baseline) Bundle metrics
|
| Current #9621 |
Baseline #9602 |
|
|---|---|---|
 Initial JS |
44.99KiB(+1.18%) |
44.46KiB |
 Initial CSS |
2.22KiB |
2.22KiB |
 Cache Invalidation |
32.89% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
228(-0.44%) |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.26%(-0.07%) |
27.28% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 2 changes 2 regressions
| Current #9621 |
Baseline #9602 |
|
|---|---|---|
| JS |
496.73KiB (+0.17%) |
495.91KiB |
| Other |
402.06KiB (+0.03%) |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:codex/web-core-frame-e... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1179 Bundle Size — 206.6KiB (0%).c3fa234(current) vs 33b124f main#1160(baseline) Bundle metrics
|
| Current #1179 |
Baseline #1160 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
192 |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.36% |
44.36% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1179 |
Baseline #1160 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
95.37KiB |
95.37KiB |
Bundle analysis report Branch PupilTong:codex/web-core-frame-e... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#314 Bundle Size — 197.79KiB (0%).c3fa234(current) vs 33b124f main#295(baseline) Bundle metrics
|
| Current #314 |
Baseline #295 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
80(-1.23%) |
81 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.31%(+0.05%) |
40.29% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #314 |
Baseline #295 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.03KiB |
52.03KiB |
Bundle analysis report Branch PupilTong:codex/web-core-frame-e... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8048 Bundle Size — 235.77KiB (0%).c3fa234(current) vs 33b124f main#8029(baseline) Bundle metrics
|
| Current #8048 |
Baseline #8029 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
197 |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.85% |
44.85% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8048 |
Baseline #8029 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.01KiB |
90.01KiB |
Bundle analysis report Branch PupilTong:codex/web-core-frame-e... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#1162 Bundle Size — 690.27KiB (0%).c3fa234(current) vs 33b124f main#1143(baseline) Bundle metrics
|
| Current #1162 |
Baseline #1143 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch PupilTong:codex/web-core-frame-e... Project dashboard
Generated by RelativeCI Documentation Report issue
2 participants
Summary
<frame>creation support in web-core PAPI and the React snapshot transform.frametolynx-viewin both TypeScript and Rust tag maps, including style selector decoding.src,data,global-props,auto-height,auto-width, andbindloadbehavior throughLynxView.Validation
cargo test -p swc_plugin_snapshot basic_full_staticcargo test -p web-core test_type_selectorpnpm turbo buildpnpm exec playwright test tests/reactlynx.spec.ts --grep api-frame --project=chromiumSummary by CodeRabbit
<frame>element on the web platform, enabling dynamic content loading from external bundles.bindloadevent for tracking frame load status with status code and message information.