feat(react): add element template alog diagnostics#2550
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
🦋 Changeset detectedLatest commit: a0e84cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
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 18.85%
Performance Changes
Comparing Footnotes
|
Web Explorer#9310 Bundle Size — 900.03KiB (0%).a0e84cf(current) vs 7abb0a9 main#9309(baseline) Bundle metrics
Bundle size by type
|
| Current #9310 |
Baseline #9309 |
|
|---|---|---|
495.9KiB |
495.9KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:slice/element-template/01 Project dashboard
Generated by RelativeCI Documentation Report issue
React External#852 Bundle Size — 680.82KiB (0%).a0e84cf(current) vs 7abb0a9 main#851(baseline) Bundle metrics
|
| Current #852 |
Baseline #851 |
|
|---|---|---|
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 Yradex:slice/element-template/01 Project dashboard
Generated by RelativeCI Documentation Report issue
React Example (Element Template)#5 Bundle Size — 198.61KiB (0%).a0e84cf(current) vs 7abb0a9 main#4(baseline) Bundle metrics
Bundle analysis report Branch Yradex:slice/element-template/01 Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#7737 Bundle Size — 225.52KiB (0%).a0e84cf(current) vs 7abb0a9 main#7736(baseline) Bundle metrics
|
| Current #7737 |
Baseline #7736 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
180 |
180 |
|
69 |
69 |
|
44.54% |
44.54% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7737 |
Baseline #7736 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.77KiB |
79.77KiB |
Bundle analysis report Branch Yradex:slice/element-template/01 Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#869 Bundle Size — 196.68KiB (0%).a0e84cf(current) vs 7abb0a9 main#868(baseline) Bundle metrics
|
| Current #869 |
Baseline #868 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
174 |
174 |
|
66 |
66 |
|
44.05% |
44.05% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #869 |
Baseline #868 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.45KiB |
85.45KiB |
Bundle analysis report Branch Yradex:slice/element-template/01 Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/react/runtime/__test__/element-template/debug/alog.test.ts (2)
84-103: ⚡ Quick winMake the tree-summary assertions less brittle (IDs + exact substrings).
This test asserts exact node IDs in the rendered string (e.g.
root#1,_et_card#2,__et_builtin_raw_text__#3) and specific substring formatting likeelementSlots[0]: [3], plusnot.toContain('elementSlots[1]').Two ways to make it more resilient:
- Derive expected
#<id>values from the createdBackgroundElementTemplateInstanceinstances instead of assuming#1/#2/#3.- Prefer verifying semantic presence/omission (e.g.,
elementSlots[0]includes thetextid) using ids computed from the instances, rather than hard-coding the slot rendering to[3].This keeps the test meaningful even if the printer’s formatting changes slightly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/runtime/__test__/element-template/debug/alog.test.ts` around lines 84 - 103, The test hard-codes node IDs and exact slot formatting making it brittle; update the assertions to compute expected ids from the created BackgroundElementTemplateInstance instances (root, card, text) and assert presence/absence semantically via printElementTemplateTreeToString output: check that the string contains `${root.id}`/`${card.id}`/`${text.id}` (or equivalent) and that elementSlots[0] references the computed text id, assert elementSlots[1] is omitted semantically (e.g., no mention of the card’s second slot or an empty slot), and verify attributeSlots contain the expected attribute values from the instances rather than exact bracket formatting. Ensure you locate and change assertions around printElementTemplateTreeToString, BackgroundElementTemplateInstance, elementSlots and attributeSlots in the test.
12-15: ⚡ Quick winAvoid coupling to
nextIdinternals inbeforeEach.This test suite hard-resets
backgroundElementTemplateInstanceManager.nextId = 0. If the manager’s internal ID strategy changes (e.g., different initial value, separate counters), these tests can start failing even when behavior is still correct.Consider adding a test-only reset helper on the manager (preferred), or at least derive the expected
#<id>suffixes from the created instances (root,card,text) rather than assuming#1/#2/#3.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/runtime/__test__/element-template/debug/alog.test.ts` around lines 12 - 15, The tests currently mutate backgroundElementTemplateInstanceManager.nextId directly in beforeEach; instead add a test-only reset method on backgroundElementTemplateInstanceManager (e.g., resetForTests or clearAndReset) that clears state and resets any internal counters, call that from beforeEach, and update tests to rely on instance identifiers returned from created instances (root, card, text) rather than hard-coding "#1/#2/#3" — locate and modify backgroundElementTemplateInstanceManager (methods like clear and nextId) to add the reset helper and update the test file to read ids from the created instance objects or the manager API when asserting suffixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/runtime/__test__/element-template/debug/alog.test.ts`:
- Around line 84-103: The test hard-codes node IDs and exact slot formatting
making it brittle; update the assertions to compute expected ids from the
created BackgroundElementTemplateInstance instances (root, card, text) and
assert presence/absence semantically via printElementTemplateTreeToString
output: check that the string contains `${root.id}`/`${card.id}`/`${text.id}`
(or equivalent) and that elementSlots[0] references the computed text id, assert
elementSlots[1] is omitted semantically (e.g., no mention of the card’s second
slot or an empty slot), and verify attributeSlots contain the expected attribute
values from the instances rather than exact bracket formatting. Ensure you
locate and change assertions around printElementTemplateTreeToString,
BackgroundElementTemplateInstance, elementSlots and attributeSlots in the test.
- Around line 12-15: The tests currently mutate
backgroundElementTemplateInstanceManager.nextId directly in beforeEach; instead
add a test-only reset method on backgroundElementTemplateInstanceManager (e.g.,
resetForTests or clearAndReset) that clears state and resets any internal
counters, call that from beforeEach, and update tests to rely on instance
identifiers returned from created instances (root, card, text) rather than
hard-coding "#1/#2/#3" — locate and modify
backgroundElementTemplateInstanceManager (methods like clear and nextId) to add
the reset helper and update the test file to read ids from the created instance
objects or the manager API when asserting suffixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 73f23fac-2237-413e-a74c-84533c1a5a5b
📒 Files selected for processing (9)
.changeset/element-template-alog-diagnostics.mdpackages/react/runtime/__test__/element-template/debug/alog.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/patch/patch-listener-alog.test.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/debug/alog.tspackages/react/runtime/src/element-template/native/patch-listener.ts
Summary by CodeRabbit
New Features
Tests
Overview
__ALOG__, so the normal runtime path does not format command streams, print background trees, or emit extra debug output.Key Points
createTemplate,setAttribute,insertNode, andremoveNode.Diagnostic Output
When
__ALOG__is enabled, compact update commands are logged in a reviewer-readable shape:{ "ops": [ { "op": "setAttribute", "targetId": 1, "attrSlotIndex": 0, "value": "next" } ], "flushOptions": {}, "flowIds": {} }The formatted output is diagnostic-only. The wire protocol remains the existing compact command stream consumed by the ET patch executor.
Checklist