fix(web-core): conditionally include Card and Component only when cardType !== 'react'.#2610
Conversation
ReactLynx bundles do not declare `Card` and `Component` in their function signature. Passing these params unconditionally shifted all subsequent parameter positions, causing `lynx` to be `undefined` and triggering `loadCard failed TypeError: Cannot read properties of undefined (reading 'performance')`. Conditionally include `Card` and `Component` only when `cardType !== 'react'`.
📝 WalkthroughWalkthroughBackground thread initialization now threads ChangesCardType-driven Parameter Injection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 docstrings
🧪 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 |
🦋 Changeset detectedLatest commit: 1c2e35e 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts (1)
113-152: ⚡ Quick winType safety degraded by
.apply()invocation pattern.The refactor from direct invocation to
(foo as Function).apply(undefined, args)(line 152) loses theBTSChunkEntrytype constraint. TypeScript can no longer verify that theargsarray matches the function signature constructed inparamNames.While this change is necessary to support variable-length parameters, consider whether the
BTSChunkEntrytype definition should be updated to reflect that the function signature is now dynamic, or add runtime assertions to verifyparamNames.length === args.lengthbefore invocation to catch mismatches during development.🤖 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/ts/client/background/background-apis/createChunkLoading.ts` around lines 113 - 152, The change to call the chunk entry via (foo as Function).apply(undefined, args) loses the BTSChunkEntry type safety and risks signature mismatches; restore safety by either updating the BTSChunkEntry type to accept a variadic signature matching paramNames or add a runtime assertion before invocation that paramNames.length === args.length (and throw/log a clear error referencing foo and paramNames on mismatch). Ensure the check is performed where args is constructed and before (foo as Function).apply(...) so callers like foo, args, paramNames and the BTSChunkEntry type are kept consistent.
🤖 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.
Inline comments:
In
`@packages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts`:
- Around line 65-76: BTSChunkEntry's function signature in BTSChunk.ts doesn't
match runtime creation in createChunkLoading.ts because paramNames/args include
Card and Component only when cardType !== 'react'; update BTSChunkEntry to
reflect both variants by adding overloads or conditional types (e.g., one
signature with Card and Component parameters and one without) so the type aligns
with the conditional runtime parameters, or alternatively change
createChunkLoading.ts to always include Card and Component in paramNames/args;
locate BTSChunkEntry, createChunkLoading.ts paramNames/args, and the cast "as
BTSChunkEntry" to apply the fix.
---
Nitpick comments:
In
`@packages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts`:
- Around line 113-152: The change to call the chunk entry via (foo as
Function).apply(undefined, args) loses the BTSChunkEntry type safety and risks
signature mismatches; restore safety by either updating the BTSChunkEntry type
to accept a variadic signature matching paramNames or add a runtime assertion
before invocation that paramNames.length === args.length (and throw/log a clear
error referencing foo and paramNames on mismatch). Ensure the check is performed
where args is constructed and before (foo as Function).apply(...) so callers
like foo, args, paramNames and the BTSChunkEntry type are kept consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a056349-93f9-45a5-b027-7e29457ec69b
📒 Files selected for processing (4)
.changeset/curvy-shrimps-stay.mdpackages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.tspackages/web-platform/web-core/ts/client/background/background-apis/createNativeApp.tspackages/web-platform/web-core/ts/client/background/background-apis/startBackgroundThread.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 17.09%
Performance Changes
Comparing Footnotes
|
React Example with Element Template#340 Bundle Size — 197.79KiB (0%).1c2e35e(current) vs 460ddbd main#331(baseline) Bundle metrics
Bundle size by type
|
| Current #340 |
Baseline #331 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
52.03KiB |
52.03KiB |
Bundle analysis report Branch fix/web-core-react-card-sandbox-... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8074 Bundle Size — 235.77KiB (0%).1c2e35e(current) vs 460ddbd main#8065(baseline) Bundle metrics
|
| Current #8074 |
Baseline #8065 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
197 |
197 |
|
80 |
80 |
|
44.85% |
44.85% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #8074 |
Baseline #8065 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
90.01KiB |
90.01KiB |
Bundle analysis report Branch fix/web-core-react-card-sandbox-... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#1187 Bundle Size — 690.27KiB (0%).1c2e35e(current) vs 460ddbd main#1178(baseline) Bundle metrics
|
| Current #1187 |
Baseline #1178 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch fix/web-core-react-card-sandbox-... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9647 Bundle Size — 901.38KiB (+0.01%).1c2e35e(current) vs 460ddbd main#9638(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch fix/web-core-react-card-sandbox-... Project dashboard Generated by RelativeCI Documentation Report issue |
React MTF Example#1205 Bundle Size — 206.65KiB (0%).1c2e35e(current) vs 460ddbd main#1196(baseline) Bundle metrics
|
| Current #1205 |
Baseline #1196 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
46.16% |
|
0 |
0 |
|
3 |
3 |
|
192 |
192 |
|
77 |
77 |
|
44.36% |
44.36% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #1205 |
Baseline #1196 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
95.42KiB |
95.42KiB |
Bundle analysis report Branch fix/web-core-react-card-sandbox-... Project dashboard
Generated by RelativeCI Documentation Report issue
Conditionally include
CardandComponentonly whencardType !== 'react'.Summary by CodeRabbit
Release Notes
Checklist