feat(react): add element template alog diagnostics#2633
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an ALOG wrapper for ElementTemplate PAPI calls (with optional profiling and formatted logging), gates it in native init via ChangesElementTemplate and Event ALOG Debug Logging
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/react/runtime/src/element-template/debug/elementPAPICall.ts (1)
15-17: Consider memory implications of unbounded template map growth.The
elementTemplateMap(line 17) grows without bounds as templates are created. For long-running debug sessions with frequent template creation, this could accumulate significant memory. Since this is debug-only code gated behind__ALOG_ELEMENT_API__, the impact is limited to development environments, but it's worth noting for operational awareness.🤖 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/react/runtime/src/element-template/debug/elementPAPICall.ts` around lines 15 - 17, The elementTemplateMap in initElementTemplatePAPICallAlog can grow unbounded; switch to a memory-safe structure or cap its size: either replace the Map<unknown,string> with a WeakMap<object,string> (use when template keys are objects so entries can be GC'd) or add a bounded cache/eviction (LRU or simple maxEntries prune) in the initElementTemplatePAPICallAlog function to prevent unbounded growth while preserving existing lookup behavior.packages/react/runtime/src/element-template/prop-adapters/event.ts (1)
14-29: ⚡ Quick winConsider adding error handling around JSON.stringify.
If
datacontains circular references or non-serializable values,JSON.stringifywill throw and prevent the event from dispatching. While this is debug-only code, it could disrupt testing or debugging sessions.🛡️ Defensive coding suggestion
if (typeof __ALOG__ !== 'undefined' && __ALOG__) { - console.alog?.( - `[ReactLynxDebug] ElementTemplate BTS received event:\n${ - JSON.stringify( - { - eventValue, - type: (data as { type?: unknown } | null)?.type, - jsFunctionName: typeof handler === 'function' ? handler.name : '', - hasHandler: typeof handler === 'function', - }, - null, - 2, - ) - }`, - ); + try { + console.alog?.( + `[ReactLynxDebug] ElementTemplate BTS received event:\n${ + JSON.stringify( + { + eventValue, + type: (data as { type?: unknown } | null)?.type, + jsFunctionName: typeof handler === 'function' ? handler.name : '', + hasHandler: typeof handler === 'function', + }, + null, + 2, + ) + }`, + ); + } catch { + console.alog?.(`[ReactLynxDebug] ElementTemplate BTS received event (stringify failed): ${eventValue}`); + } }🤖 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/react/runtime/src/element-template/prop-adapters/event.ts` around lines 14 - 29, Wrap the debug JSON.stringify call in a try/catch so circular or non-serializable data in eventValue/data doesn't throw and block dispatch; inside the catch build a safe fallback string (include a short message and the error.message) and still call console.alog. Update the block that checks __ALOG__ and calls console.alog to use the safe-serialized payload for eventValue, type ((data as { type?: unknown } | null)?.type), jsFunctionName/hasHandler (handler) so the logger always runs even if JSON.stringify fails.
🤖 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/react/runtime/src/element-template/debug/elementPAPICall.ts`:
- Around line 26-53: The profiler may be left open if callElementTemplatePAPI
throws; wrap the call to callElementTemplatePAPI(...) and subsequent profiling
logic in a try/finally so that profileEnd() is always invoked when
profileStart(...) ran; specifically, in the
globalWithIndex[elementTemplatePAPIName] handler, call profileStart(...) before
invoking callElementTemplatePAPI, assign the result inside the try block, and
call profileEnd() in the finally block (still only when __PROFILE__ is enabled)
so that profileEnd is executed even on exceptions while preserving existing
behavior for elementTemplateMap, console.alog, and the returned result.
---
Nitpick comments:
In `@packages/react/runtime/src/element-template/debug/elementPAPICall.ts`:
- Around line 15-17: The elementTemplateMap in initElementTemplatePAPICallAlog
can grow unbounded; switch to a memory-safe structure or cap its size: either
replace the Map<unknown,string> with a WeakMap<object,string> (use when template
keys are objects so entries can be GC'd) or add a bounded cache/eviction (LRU or
simple maxEntries prune) in the initElementTemplatePAPICallAlog function to
prevent unbounded growth while preserving existing lookup behavior.
In `@packages/react/runtime/src/element-template/prop-adapters/event.ts`:
- Around line 14-29: Wrap the debug JSON.stringify call in a try/catch so
circular or non-serializable data in eventValue/data doesn't throw and block
dispatch; inside the catch build a safe fallback string (include a short message
and the error.message) and still call console.alog. Update the block that checks
__ALOG__ and calls console.alog to use the safe-serialized payload for
eventValue, type ((data as { type?: unknown } | null)?.type),
jsFunctionName/hasHandler (handler) so the logger always runs even if
JSON.stringify fails.
🪄 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: d26f7009-afa5-41e1-9fcf-16f6e7942982
📒 Files selected for processing (6)
packages/react/runtime/__test__/element-template/debug/elementPAPICall.test.tspackages/react/runtime/__test__/element-template/native/index.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/event.test.tspackages/react/runtime/src/element-template/debug/elementPAPICall.tspackages/react/runtime/src/element-template/native/index.tspackages/react/runtime/src/element-template/prop-adapters/event.ts
Merging this PR will improve performance by ×4.2
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | basic-performance-large-css |
68.5 ms | 16.1 ms | ×4.2 |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Yradex:wt/et-alog-debug (64a00d9) with main (a973c54)
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. ↩
React External#1315 Bundle Size — 693.04KiB (0%).64a00d9(current) vs 66f6ecf main#1314(baseline) Bundle metrics
|
| Current #1315 |
Baseline #1314 |
|
|---|---|---|
 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 Yradex:wt/et-alog-debug Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1334 Bundle Size — 207.46KiB (0%).64a00d9(current) vs 66f6ecf main#1333(baseline) Bundle metrics
|
| Current #1334 |
Baseline #1333 |
|
|---|---|---|
| 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.38% |
44.38% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1334 |
Baseline #1333 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.23KiB |
96.23KiB |
Bundle analysis report Branch Yradex:wt/et-alog-debug Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8201 Bundle Size — 236.51KiB (0%).64a00d9(current) vs 66f6ecf main#8200(baseline) Bundle metrics
|
| Current #8201 |
Baseline #8200 |
|
|---|---|---|
| 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.87% |
44.87% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8201 |
Baseline #8200 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.75KiB |
90.75KiB |
Bundle analysis report Branch Yradex:wt/et-alog-debug Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9775 Bundle Size — 901.38KiB (0%).64a00d9(current) vs a973c54 main#9764(baseline) Bundle metrics
|
| Current #9775 |
Baseline #9764 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
 Modules |
230(+0.44%) |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.21%(-0.04%) |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9775 |
Baseline #9764 |
|
|---|---|---|
| JS |
497.1KiB |
497.1KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:wt/et-alog-debug Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#467 Bundle Size — 199.83KiB (0%).64a00d9(current) vs 66f6ecf main#466(baseline) Bundle metrics
|
| Current #467 |
Baseline #466 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
88(+3.53%) |
85 |
 Duplicate Modules |
26(+4%) |
25 |
| Duplicate Code |
40.03%(+0.15%) |
39.97% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #467 |
Baseline #466 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
54.08KiB |
54.08KiB |
Bundle analysis report Branch Yradex:wt/et-alog-debug Project dashboard
Generated by RelativeCI Documentation Report issue
3 participants
Summary by CodeRabbit
New Features
Tests
Chores
Overview
Adds Element Template-only diagnostic logging behind the existing ALog flags so native PAPI calls and background event dispatch can be inspected without adding a private debug publish hook or touching Snapshot wiring.
Key Points
REACT_ALOG_ELEMENT_APIinstalls an ET native PAPI wrapper from the ET native entry, covering create/set/insert/remove/serialize calls with readable template ref labels.REACT_ALOGlogs background event dispatch metadata after handler lookup: eventValue, event type, resolved handler name, and whether a handler exists.__etDebugPublishEventor new public runtime exports.Checklist