fix: web-core-wasm triggerEvent & lazy component for special DSL usage#2399
Merged
PupilTong merged 2 commits intolynx-family:mainfrom Mar 30, 2026
Conversation
… tests for component query evaluation
🦋 Changeset detectedLatest commit: bf1c891 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 |
Contributor
📝 WalkthroughWalkthroughThis PR introduces event bubble support by adding an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/web-platform/web-core-e2e/tests/web-core.test.ts (2)
186-188: Minor: Redundantunhandledrejectionlistener pattern.The
unhandledrejectionlistener is added inside everypage.evaluatecall but only captures rejections that occur before the callback resolves. If__QueryComponentalways invokes the callback (even on error), this listener may never trigger. Consider whether this pattern is necessary or if it could be simplified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core-e2e/tests/web-core.test.ts` around lines 186 - 188, The unhandledrejection listener added via globalThis.addEventListener('unhandledrejection', ...) inside each page.evaluate is redundant because __QueryComponent appears to always invoke the callback (even on error), so the listener will rarely fire and is added repeatedly; remove the per-evaluation listener and instead rely on the callback path from __QueryComponent to report errors (or add a single global unhandledrejection handler once outside page.evaluate if you need to capture asynchronous rejections not reported by the callback), ensuring you reference and use the existing callback resolution logic for error propagation.
175-241: Good test coverage, but consider adding a test for intentionalnull/undefinedreturns.The two tests cover:
- Absent
processEvalResult→ fallback to original export- Present
processEvalResultreturning a value → use that valueHowever, given the concern raised about the
??operator inLynxViewInstance.ts, consider adding a third test case whereprocessEvalResultintentionally returnsnullorundefinedto verify the expected behavior:📝 Suggested additional test case
test('__QueryComponent with processEvalResult returning null', async ({ page, browserName }) => { test.skip(browserName === 'firefox'); await goto(page); await page.evaluate(() => { (globalThis as any).runtime.processEvalResult = () => null; }); const evalResult = await page.evaluate(async () => { return new Promise((resolve) => { globalThis.addEventListener('unhandledrejection', (e) => { resolve({ error: e.reason?.message ?? String(e.reason) }); }); globalThis.runtime.__QueryComponent( '/resources/mock-component.json', (res: any) => { resolve(res); }, ); }); }); expect(evalResult).toMatchObject({ code: 0 }); // This assertion depends on intended behavior: // If null is a valid intentional return, expect null // If null should fallback, expect { mockResult: 'MOCKED_EVAL_RESULT' } expect((evalResult as any).data.evalResult).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core-e2e/tests/web-core.test.ts` around lines 175 - 241, Add a third E2E test in packages/web-platform/web-core-e2e/tests/web-core.test.ts named "__QueryComponent with processEvalResult returning null" that mirrors the existing two tests: skip for firefox, call goto(page), set (globalThis as any).runtime.processEvalResult = () => null before invoking globalThis.runtime.__QueryComponent('/resources/mock-component.json', ...), await the resolved evalResult, and assert code === 0; then assert (evalResult as any).data.evalResult is either null or the original export depending on the intended behavior in LynxViewInstance.ts (choose and document which behavior you expect). Ensure the test also covers undefined by adding a similar case or changing the stub to return undefined to validate both scenarios..changeset/fix-query-component-processevalresult.md (1)
1-5: Changeset description may be misleading.The description says "when
processEvalResultis absent" but the implementation uses??which also triggers whenprocessEvalResultreturnsnullorundefined. If the regression concern inLynxViewInstance.tsis addressed, this description would be accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/fix-query-component-processevalresult.md around lines 1 - 5, The changeset description is misleading because the code uses the nullish coalescing operator (processEvalResult ?? fallback) which also triggers when processEvalResult returns null or undefined, not only when the symbol is absent; update the changeset text to say "when processEvalResult is absent or returns null/undefined during queryComponent execution" and/or change the implementation in the queryComponent code to use an explicit undefined check (e.g., check processEvalResult === undefined) so only a truly missing value (not null) falls back—refer to processEvalResult, queryComponent, and LynxViewInstance.ts when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts`:
- Around line 249-252: The current assignment uses the nullish coalescing
operator and can overwrite intentional null/undefined returns from
mainThreadGlobalThis.processEvalResult; change the logic in LynxViewInstance
where lepusRootChunkExport is set so it only invokes processEvalResult if
mainThreadGlobalThis.processEvalResult exists and then assigns its return value
directly to lepusRootChunkExport (do not use ?? fallback), i.e., check for
presence of mainThreadGlobalThis.processEvalResult before calling it and let its
result (including null or undefined) be the assigned value.
---
Nitpick comments:
In @.changeset/fix-query-component-processevalresult.md:
- Around line 1-5: The changeset description is misleading because the code uses
the nullish coalescing operator (processEvalResult ?? fallback) which also
triggers when processEvalResult returns null or undefined, not only when the
symbol is absent; update the changeset text to say "when processEvalResult is
absent or returns null/undefined during queryComponent execution" and/or change
the implementation in the queryComponent code to use an explicit undefined check
(e.g., check processEvalResult === undefined) so only a truly missing value (not
null) falls back—refer to processEvalResult, queryComponent, and
LynxViewInstance.ts when making the change.
In `@packages/web-platform/web-core-e2e/tests/web-core.test.ts`:
- Around line 186-188: The unhandledrejection listener added via
globalThis.addEventListener('unhandledrejection', ...) inside each page.evaluate
is redundant because __QueryComponent appears to always invoke the callback
(even on error), so the listener will rarely fire and is added repeatedly;
remove the per-evaluation listener and instead rely on the callback path from
__QueryComponent to report errors (or add a single global unhandledrejection
handler once outside page.evaluate if you need to capture asynchronous
rejections not reported by the callback), ensuring you reference and use the
existing callback resolution logic for error propagation.
- Around line 175-241: Add a third E2E test in
packages/web-platform/web-core-e2e/tests/web-core.test.ts named
"__QueryComponent with processEvalResult returning null" that mirrors the
existing two tests: skip for firefox, call goto(page), set (globalThis as
any).runtime.processEvalResult = () => null before invoking
globalThis.runtime.__QueryComponent('/resources/mock-component.json', ...),
await the resolved evalResult, and assert code === 0; then assert (evalResult as
any).data.evalResult is either null or the original export depending on the
intended behavior in LynxViewInstance.ts (choose and document which behavior you
expect). Ensure the test also covers undefined by adding a similar case or
changing the stub to return undefined to validate both scenarios.
🪄 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: 7f49eea9-7bd7-4660-a041-230380a0c687
📒 Files selected for processing (12)
.changeset/add-event-bubble-support.md.changeset/fix-query-component-processevalresult.mdpackages/web-platform/web-core-e2e/resources/mock-component.jsonpackages/web-platform/web-core-e2e/tests/web-core.test.tspackages/web-platform/web-core/binary/client/client.d.tspackages/web-platform/web-core/binary/client/client_bg.wasm.d.tspackages/web-platform/web-core/binary/client_legacy/client.d.tspackages/web-platform/web-core/binary/client_legacy/client_bg.wasm.d.tspackages/web-platform/web-core/src/main_thread/client/element_apis/event_apis.rspackages/web-platform/web-core/tests/element-apis.spec.tspackages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts
Merging this PR will improve performance by 26.99%
Performance Changes
Comparing 
Footnotes
|
React Example#6951 Bundle Size — 237.89KiB (0%).bf1c891(current) vs 78493b4 main#6942(baseline) Bundle metrics
|
| Current #6951 |
Baseline #6942 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
180 |
180 |
| Duplicate Modules |
71 |
71 |
| Duplicate Code |
46.4% |
46.4% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6951 |
Baseline #6942 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
92.13KiB |
92.13KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-lazy-componen... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#85 Bundle Size — 207.47KiB (0%).bf1c891(current) vs 78493b4 main#76(baseline) Bundle metrics
|
| Current #85 |
Baseline #76 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
174 |
174 |
| Duplicate Modules |
68 |
68 |
| Duplicate Code |
46.09% |
46.09% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #85 |
Baseline #76 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.24KiB |
96.24KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-lazy-componen... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#8529 Bundle Size — 728.84KiB (+0.03%).bf1c891(current) vs 78493b4 main#8520(baseline) Bundle metrics
|
| Current #8529 |
Baseline #8520 |
|
|---|---|---|
 Initial JS |
43.31KiB(~+0.01%) |
43.3KiB |
| Initial CSS |
2.16KiB |
2.16KiB |
 Cache Invalidation |
48.73% |
0% |
| Chunks |
8 |
8 |
| Assets |
10 |
10 |
| Modules |
150(+1.35%) |
148 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
34.68%(-0.06%) |
34.7% |
| Packages |
3 |
3 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 2 changes 2 regressions
| Current #8529 |
Baseline #8520 |
|
|---|---|---|
| Other |
384.62KiB (+0.06%) |
384.4KiB |
| JS |
342.07KiB (~+0.01%) |
342.04KiB |
| CSS |
2.16KiB |
2.16KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-lazy-componen... Project dashboard
Generated by RelativeCI Documentation Report issue
Sherry-hue
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Bug Fixes
queryComponentfallback behavior when processing evaluation results is unavailableTests
Checklist