Skip to content

feat(react): add richer profiling traces for hooks and hydrate#2235

Merged
Yradex merged 20 commits intolynx-family:mainfrom
Yradex:wt/render-trace-20260211
Mar 3, 2026
Merged

feat(react): add richer profiling traces for hooks and hydrate#2235
Yradex merged 20 commits intolynx-family:mainfrom
Yradex:wt/render-trace-20260211

Conversation

@Yradex
Copy link
Copy Markdown
Collaborator

@Yradex Yradex commented Feb 13, 2026

Summary by CodeRabbit

  • New Features

    • Enhanced profiling: lower overhead when disabled, richer trace events for hooks, state updates, hydration/patches, and flow ID propagation; improved source attribution for better trace context.
  • Tests

    • Expanded coverage for profiling utilities, hook traces, vnode source mapping, and hydrate/background snapshot scenarios.
  • Documentation

    • API docs updated: exported hook signatures for useEffect/useLayoutEffect changed to const arrow types.

Summary

This PR improves React runtime profiling observability and trace attribution while keeping low overhead when profiling is disabled.

What Changed

  • Added profiling traces for hooks:
  • useEffect / useLayoutEffect entry
  • effect callback and cleanup phases
  • useState setter
  • Added profileFlowId support in debug profile utilities and hooked flow IDs into hook traces.
  • Added profiling around background snapshot hydrate patch operations with richer args (snapshot id/type, dynamic part index, value type, source).
  • Added vnode source capture and wiring for trace source attribution in dev.
  • Added and reorganized debug tests for:
  • profile module behavior
  • hook profiling behavior
  • vnode source capture/move/clear behavior
  • background snapshot hydrate profile/non-profile branches and source args
  • Added changeset: .changeset/free-dragons-mate.md.

Profiling Usage

Enable Profiling recording first, then enter the target page, so the trace includes full render/hydrate phases.

Testing

  • pnpm exec vitest run __test__/debug/*.test.*
  • pnpm exec vitest run --coverage.enabled --coverage.include=src/hooks/react.ts --coverage.include=src/backgroundSnapshot.ts --coverage.reporter=text

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).
  • Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

@Yradex Yradex requested review from HuJean and hzy as code owners February 13, 2026 06:26
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 13, 2026

🦋 Changeset detected

Latest commit: b96b59a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lynx-js/react Patch

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a streamlined profiling core, installs background/profile hooks, captures vnode source locations, instruments hydration and background-snapshot patch paths with richer trace args, updates imports to the new profile API, and adds comprehensive tests for profiling and vnode-source behaviors.

Changes

Cohort / File(s) Summary
Profiling core
packages/react/runtime/src/debug/profile.ts, packages/react/runtime/src/debug/utils.ts
Introduce isProfiling, profileStart, profileEnd, profileFlowId in profile.ts and remove profileStart/profileEnd exports from utils.ts; provide no-op fallbacks and simplified bindings.
Profile hook installer
packages/react/runtime/src/debug/profileHooks.ts
Add initProfileHook() that wires background profiling hooks (setState, diff, diffed, commit, render) and propagates per-instance flowIds when profiling APIs are available.
Hook-level profiling
packages/react/runtime/src/hooks/react.ts, packages/react/etc/react.api.md
Add profiling wrappers for useEffect/useLayoutEffect and a profiled useState setter; exports now conditionally alias hooks to profiled variants when isProfiling; update API typing for exported hooks.
VNode source tracking
packages/react/runtime/src/debug/vnodeSource.ts, packages/react/runtime/__test__/debug/vnodeSource.test.ts
New module to capture/format vnode __source into a DOM-id → source map; expose setupVNodeSourceHook, getSnapshotVNodeSource, moveSnapshotVNodeSource, clearSnapshotVNodeSource; add tests for formatting, move, and clear.
Hydration & background snapshot profiling
packages/react/runtime/src/backgroundSnapshot.ts, packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx
Wrap hydrate/patch emission and reconstruction calls with guarded profileStart/profileEnd; attach richer profiling args (snapshot id/type, dynamicPartIndex, valueType, vnode source) and add tests validating trace emission and patch decoding.
Integration & telemetry
packages/react/runtime/src/lynx.ts, packages/react/runtime/src/lynx/tt.ts, packages/react/runtime/src/snapshot.ts
Switch runtime to isProfiling flow, call setupVNodeSourceHook() in DEV when profiling, wire vnode source into telemetry payloads, and call move/clear vnode-source helpers during snapshot lifecycle.
Import consolidation
multiple runtime files
packages/react/runtime/src/{hydrate,lynx-api,lynx/env,lynx/tt,listUpdateInfo}.ts, packages/react/runtime/src/lifecycle/{destroy,event/jsReady,patch/commit,reload,render}.ts
Update many imports to use ./debug/profile.js instead of ./debug/utils.js (no functional changes beyond source module swap).
Tests & validation
packages/react/runtime/__test__/debug/{backgroundSnapshot-profile,profile-module,profile,react-hooks-profile,hooks-invalid-args}.test.*
Add and update tests covering profile module init/fallback, hook profiling (flowIds, stacks, setter traces), vnode source mapping, hydrate profiling traces, and a hook-args warning expectation fix.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • HuJean
  • hzy

Poem

🐇 I hopped through traces, quick and bright,
FlowIds stitched the day and night.
VNodes told me file and line,
Hydration timed in tidy rhyme.
Tests thump paws — the runtime feels light.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes: adding enhanced profiling traces for React hooks and hydration. It accurately reflects the primary objectives of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 13, 2026

Codecov Report

❌ Patch coverage is 99.34534% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/react/runtime/src/lynx.ts 0.00% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@relativeci
Copy link
Copy Markdown

relativeci bot commented Feb 13, 2026

Web Explorer

#7820 Bundle Size — 383.74KiB (0%).

b96b59a(current) vs 09929fb main#7815(baseline)

Bundle metrics  Change 1 change
                 Current
#7820
     Baseline
#7815
No change  Initial JS 154.88KiB 154.88KiB
No change  Initial CSS 35.06KiB 35.06KiB
No change  Cache Invalidation 0% 0%
No change  Chunks 8 8
No change  Assets 8 8
Change  Modules 238(+0.85%) 236
No change  Duplicate Modules 16 16
No change  Duplicate Code 2.99% 2.99%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#7820
     Baseline
#7815
No change  JS 252.83KiB 252.83KiB
No change  Other 95.85KiB 95.85KiB
No change  CSS 35.06KiB 35.06KiB

Bundle analysis reportBranch Yradex:wt/render-trace-20260211Project dashboard


Generated by RelativeCIDocumentationReport issue

@Yradex Yradex force-pushed the wt/render-trace-20260211 branch from 5ea9eca to b108dd8 Compare February 24, 2026 03:14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
packages/react/runtime/src/backgroundSnapshot.ts (1)

453-631: Consider a helper to reduce profiling-branch duplication.

Each __globalSnapshotPatch!.push(...) call is duplicated in both the if (shouldProfile) { profileStart; try { push } finally { profileEnd } } else { push } branches — this pattern appears ~5 times. The duplication makes future changes to the push arguments error-prone.

A small inline helper would eliminate the repetition without measurable overhead:

♻️ Suggested helper (illustrative)
function tracedPush(
  name: string,
  args: Record<string, string>,
  ...pushArgs: Parameters<typeof __globalSnapshotPatch.push>
): void {
  if (shouldProfile) {
    profileStart(name, { args });
  }
  __globalSnapshotPatch!.push(...pushArgs);
  if (shouldProfile) {
    profileEnd();
  }
}

Note: this drops the try/finally around push. If you need exception safety for profileEnd, keep the try/finally inside the helper.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/backgroundSnapshot.ts` around lines 453 - 631,
Multiple places duplicate the profiling branch around
__globalSnapshotPatch!.push; add a small helper (e.g., tracedPush) that accepts
the profiling name, profiling args, and the push arguments, moves the
profileStart/profileEnd logic (with try/finally around the push to preserve
exception-safety) into the helper, and call tracedPush instead of duplicating
the if (shouldProfile) { profileStart; try { push } finally { profileEnd } }
else { push } pattern. Replace each occurrence in this file (references: the
SetAttribute blocks, the RemoveChild/InsertBefore blocks inside diffArrayAction,
and any other push sites inside helper/reconstructInstanceTree callbacks) with
tracedPush(...) so the push call and its arguments (SnapshotOperation.*, ids,
keys, values, target ids) are passed through unchanged while consolidating
profiling logic.
packages/react/runtime/src/lynx/tt.ts (1)

207-234: snapshotId computed unconditionally; ALOG block still re-derives it inline.

snapshotId (line 207) is extracted before the __PROFILE__ guard, so it runs on every event even when profiling is disabled. The __ALOG__ block at line 232 then redundantly re-computes Number(handlerName.split(':')[0]) instead of reusing it.

♻️ Suggested refactor
-  const snapshotId = Number(handlerName.split(':')[0]);
   const eventHandler = backgroundSnapshotInstanceManager.getValueBySign(
     handlerName,
   ) as ((data: unknown) => void) | undefined;

   if (__PROFILE__) {
+    const snapshotId = Number(handlerName.split(':')[0]);
     profileStart(`ReactLynx::publishEvent`, {
       args: {
         handlerName,
         type: data.type,
-        snapshotInstanceType: backgroundSnapshotInstanceManager.values.get(
-          snapshotId,
-        )?.type ?? '',
+        snapshotInstanceType: backgroundSnapshotInstanceManager.values.get(snapshotId)?.type ?? '',
         source: getSnapshotVNodeSource(snapshotId) ?? '',
         jsFunctionName: eventHandler?.name ?? '',
       },
     });
   }
   if (typeof __ALOG__ !== 'undefined' && __ALOG__) {
+    const snapshotId = Number(handlerName.split(':')[0]);
     console.alog?.(
       `[ReactLynxDebug] BTS received event:\n` + JSON.stringify(
         {
           handlerName,
           type: data.type,
-          snapshotInstanceType: backgroundSnapshotInstanceManager.values.get(
-            Number(handlerName.split(':')[0]),
-          )?.type ?? '',
+          snapshotInstanceType: backgroundSnapshotInstanceManager.values.get(snapshotId)?.type ?? '',
           jsFunctionName: eventHandler?.name ?? '',
         },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/lynx/tt.ts` around lines 207 - 234, snapshotId is
computed unconditionally and then recomputed inside the __ALOG__ block; to fix,
defer computing Number(handlerName.split(':')[0]) until needed and reuse the
local snapshotId: move the snapshotId extraction behind the __PROFILE__ and
__ALOG__ guards (or compute it lazily) and update the __ALOG__ block to
reference snapshotId instead of recomputing; ensure eventHandler and
backgroundSnapshotInstanceManager lookups still use the same snapshotId variable
(references: snapshotId, eventHandler, __PROFILE__, __ALOG__,
backgroundSnapshotInstanceManager).
packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx (1)

34-48: Fail fast on unknown snapshot operations.

Silently breaking on unknown ops can mask new/invalid patch sequences. Throwing will make test failures more explicit.

🛠️ Suggested change
-    if (!params) {
-      break;
-    }
+    if (!params) {
+      throw new Error(`Unknown snapshot op: ${op}`);
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx`
around lines 34 - 48, The decodePatch function currently stops processing
silently when it encounters an unknown operation because it checks
SnapshotOperationParams[op]?.params and breaks when falsy; modify decodePatch
(the while loop logic) to throw a descriptive error when params are missing for
an op (include the op value and current index) instead of breaking, so
unknown/invalid SnapshotOperationParams entries fail tests loudly.
.changeset/free-dragons-mate.md (1)

8-12: Vary bullet verbs to reduce repetition.

The list reads smoother if the bullets don’t all start with “Add.” Consider mixing verbs for readability.

✏️ Optional wording tweak
- - Add trace events for `useEffect` / `useLayoutEffect` hook entry, callback, and cleanup phases.
- - Add trace event for `useState` setter calls.
- - Add `profileFlowId` support in debug profile utilities and attach flow IDs to related hook traces.
- - Add hydrate/background snapshot profiling around patch operations with richer args (e.g. snapshot id/type, dynamic part index, value type, and source when available).
- - Capture vnode source mapping in dev and use it in profiling args to improve trace attribution.
- - Add/expand debug test coverage for profile utilities, hook profiling behavior, vnode source mapping, and hydrate profiling branches.
+ - Add trace events for `useEffect` / `useLayoutEffect` hook entry, callback, and cleanup phases.
+ - Introduce a trace event for `useState` setter calls.
+ - Wire `profileFlowId` support in debug profile utilities and attach flow IDs to related hook traces.
+ - Instrument hydrate/background snapshot profiling around patch operations with richer args (e.g. snapshot id/type, dynamic part index, value type, and source when available).
+ - Capture vnode source mapping in dev and use it in profiling args to improve trace attribution.
+ - Expand debug test coverage for profile utilities, hook profiling behavior, vnode source mapping, and hydrate profiling branches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.changeset/free-dragons-mate.md around lines 8 - 12, The bulleted changelog
lines all begin with "Add", making the list repetitive; please rewrite the
bullets to vary verbs while keeping the same meaning—e.g., use "Record" for hook
trace events (useEffect/useLayoutEffect), "Log" or "Attach" for useState setter
traces and profileFlowId behavior, "Support" for profileFlowId in debug profile
utilities, "Wrap" or "Instrument" for hydrate/background snapshot profiling, and
"Capture" for vnode source mapping—ensure each bullet still references the
original concepts (useEffect/useLayoutEffect, useState setter, profileFlowId,
hydrate/background snapshot profiling, vnode source mapping) so intent remains
clear.
🤖 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/react/runtime/src/debug/profileHooks.ts`:
- Around line 108-113: Update the inline comment that follows the profileEnd()
call to reference the correct hook: replace the incorrect "for options[DIFF]"
comment with "for options[DIFF2]" so it matches the trace opened in the DIFF2
hook (see profileEnd() usage in the hook(options, DIFFED, (old, vnode) => { ...
}) block and the corresponding profile start in the DIFF2 hook).
- Around line 159-172: The DIFFED hook callback can leave the Symbol property
sPatchLength on vnode if __globalSnapshotPatch was truthy at DIFF but became
falsy by DIFFED; move the cleanup so delete vnode[sPatchLength] always executes
regardless of the __globalSnapshotPatch check while preserving the existing
profileMark call (which should remain inside the typeof vnode.type ===
'function' && __globalSnapshotPatch guard) — update the hook(DIFFED, (old,
vnode: PatchedVNode) => { ... }) callback so the conditional only wraps the
profileMark/getDisplayName logic and the delete vnode[sPatchLength] runs
unconditionally before invoking old?.(vnode).

In `@packages/react/runtime/src/debug/vnodeSource.ts`:
- Around line 28-34: The formatSource logic currently returns only fileName when
lineNumber is missing even if source.columnNumber exists; update the branching
in formatSource to add an explicit branch that returns fileName:columnNumber (or
fileName::columnNumber or another clear format you choose) when typeof
source.columnNumber === 'number' and typeof source.lineNumber !== 'number',
referencing source.fileName, source.lineNumber and source.columnNumber so the
column-only case is not silently dropped; alternatively, add a comment
documenting intentional omission if you prefer not to emit column-only
information.

---

Nitpick comments:
In @.changeset/free-dragons-mate.md:
- Around line 8-12: The bulleted changelog lines all begin with "Add", making
the list repetitive; please rewrite the bullets to vary verbs while keeping the
same meaning—e.g., use "Record" for hook trace events
(useEffect/useLayoutEffect), "Log" or "Attach" for useState setter traces and
profileFlowId behavior, "Support" for profileFlowId in debug profile utilities,
"Wrap" or "Instrument" for hydrate/background snapshot profiling, and "Capture"
for vnode source mapping—ensure each bullet still references the original
concepts (useEffect/useLayoutEffect, useState setter, profileFlowId,
hydrate/background snapshot profiling, vnode source mapping) so intent remains
clear.

In `@packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx`:
- Around line 34-48: The decodePatch function currently stops processing
silently when it encounters an unknown operation because it checks
SnapshotOperationParams[op]?.params and breaks when falsy; modify decodePatch
(the while loop logic) to throw a descriptive error when params are missing for
an op (include the op value and current index) instead of breaking, so
unknown/invalid SnapshotOperationParams entries fail tests loudly.

In `@packages/react/runtime/src/backgroundSnapshot.ts`:
- Around line 453-631: Multiple places duplicate the profiling branch around
__globalSnapshotPatch!.push; add a small helper (e.g., tracedPush) that accepts
the profiling name, profiling args, and the push arguments, moves the
profileStart/profileEnd logic (with try/finally around the push to preserve
exception-safety) into the helper, and call tracedPush instead of duplicating
the if (shouldProfile) { profileStart; try { push } finally { profileEnd } }
else { push } pattern. Replace each occurrence in this file (references: the
SetAttribute blocks, the RemoveChild/InsertBefore blocks inside diffArrayAction,
and any other push sites inside helper/reconstructInstanceTree callbacks) with
tracedPush(...) so the push call and its arguments (SnapshotOperation.*, ids,
keys, values, target ids) are passed through unchanged while consolidating
profiling logic.

In `@packages/react/runtime/src/lynx/tt.ts`:
- Around line 207-234: snapshotId is computed unconditionally and then
recomputed inside the __ALOG__ block; to fix, defer computing
Number(handlerName.split(':')[0]) until needed and reuse the local snapshotId:
move the snapshotId extraction behind the __PROFILE__ and __ALOG__ guards (or
compute it lazily) and update the __ALOG__ block to reference snapshotId instead
of recomputing; ensure eventHandler and backgroundSnapshotInstanceManager
lookups still use the same snapshotId variable (references: snapshotId,
eventHandler, __PROFILE__, __ALOG__, backgroundSnapshotInstanceManager).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ea9eca and b108dd8.

📒 Files selected for processing (27)
  • .changeset/free-dragons-mate.md
  • packages/react/etc/react.api.md
  • packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx
  • packages/react/runtime/__test__/debug/hooks-invalid-args.test.jsx
  • packages/react/runtime/__test__/debug/profile-module.test.ts
  • packages/react/runtime/__test__/debug/profile.test.jsx
  • packages/react/runtime/__test__/debug/react-hooks-profile.test.jsx
  • packages/react/runtime/__test__/debug/vnodeSource.test.ts
  • packages/react/runtime/src/alog/elementPAPICall.ts
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/debug/profile.ts
  • packages/react/runtime/src/debug/profileHooks.ts
  • packages/react/runtime/src/debug/utils.ts
  • packages/react/runtime/src/debug/vnodeSource.ts
  • packages/react/runtime/src/hooks/react.ts
  • packages/react/runtime/src/hydrate.ts
  • packages/react/runtime/src/lifecycle/destroy.ts
  • packages/react/runtime/src/lifecycle/event/jsReady.ts
  • packages/react/runtime/src/lifecycle/patch/commit.ts
  • packages/react/runtime/src/lifecycle/reload.ts
  • packages/react/runtime/src/lifecycle/render.ts
  • packages/react/runtime/src/listUpdateInfo.ts
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/runtime/src/lynx.ts
  • packages/react/runtime/src/lynx/env.ts
  • packages/react/runtime/src/lynx/tt.ts
  • packages/react/runtime/src/snapshot.ts
💤 Files with no reviewable changes (1)
  • packages/react/runtime/src/debug/utils.ts
🚧 Files skipped from review as they are similar to previous changes (14)
  • packages/react/runtime/src/lifecycle/destroy.ts
  • packages/react/runtime/src/lifecycle/patch/commit.ts
  • packages/react/runtime/test/debug/profile-module.test.ts
  • packages/react/runtime/test/debug/hooks-invalid-args.test.jsx
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/etc/react.api.md
  • packages/react/runtime/src/hydrate.ts
  • packages/react/runtime/src/snapshot.ts
  • packages/react/runtime/src/lifecycle/reload.ts
  • packages/react/runtime/src/alog/elementPAPICall.ts
  • packages/react/runtime/test/debug/profile.test.jsx
  • packages/react/runtime/src/lynx.ts
  • packages/react/runtime/test/debug/vnodeSource.test.ts
  • packages/react/runtime/src/listUpdateInfo.ts

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Feb 24, 2026

Merging this PR will degrade performance by 10.09%

⚡ 1 improved benchmark
❌ 3 regressed benchmarks
✅ 59 untouched benchmarks
⏩ 3 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
transform 1000 view elements 40.5 ms 44.8 ms -9.63%
basic-performance-div-10000 484.2 ms 459 ms +5.47%
006-static-raw-text-hydrate 1.6 ms 1.8 ms -8.63%
002-hello-reactLynx-hydrate 1.5 ms 1.6 ms -10.09%

Comparing Yradex:wt/render-trace-20260211 (b96b59a) with main (09929fb)

Open in CodSpeed

Footnotes

  1. 3 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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/react/runtime/src/debug/profileHooks.ts (1)

159-172: The sPatchLength cleanup concern from the prior review appears addressed.

In the current code, delete vnode[sPatchLength] (line 169) executes for all function-type vnodes regardless of __globalSnapshotPatch's truthiness at DIFFED time, while the profileMark is correctly guarded. This matches the fix previously suggested.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/debug/profileHooks.ts` around lines 159 - 172, The
cleanup currently deletes vnode[sPatchLength] for all function-type vnodes even
when __globalSnapshotPatch is falsy; to fix, move the delete vnode[sPatchLength]
inside the same conditional that calls profileMark (the if
(__globalSnapshotPatch && vnode[sPatchLength] === __globalSnapshotPatch.length)
block) in the hook callback so sPatchLength is removed only when the
snapshot-patch condition is met (references: sPatchLength,
__globalSnapshotPatch, profileMark, the hook(options, DIFFED, (old, vnode:
PatchedVNode) => { ... }) callback).
🧹 Nitpick comments (2)
packages/react/runtime/src/debug/profileHooks.ts (2)

63-79: setState hook calls old first, then profiles — the trace reflects post-merge state, which is intentional but worth a brief doc note.

Because old?.call(this, state, callback) runs before the profileMark, this[NEXT_STATE] already contains the merged state at profiling time. This is fine for capturing "changed keys," but a one-line comment clarifying the ordering would help future readers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/debug/profileHooks.ts` around lines 63 - 79, The
setState hook currently calls old?.call(this, state, callback) before invoking
profileMark, so this[NEXT_STATE] reflects the post-merge state; add a one-line
comment in the setState hook (near the old?.call(...) and profileMark(...)
calls) explaining that the ordering is intentional to capture the
merged/after-state for buildSetStateProfileMarkArgs and that DIRTY,
sFlowID/profileFlowId are used to gate/identify the trace. Ensure the comment
references setState, NEXT_STATE, profileMark, and buildSetStateProfileMarkArgs
so future readers understand why we profile after calling the original setState.

42-61: EMPTY_OBJ is re-allocated on every setState profiling call.

EMPTY_OBJ only serves as a fallback for the nullish-coalescing assignments and is never mutated. Hoisting it outside the function avoids a needless allocation per call.

♻️ Suggested diff
+    const EMPTY_OBJ = {};
+
     function buildSetStateProfileMarkArgs(
       currentState: Record<string, unknown>,
       nextState: Record<string, unknown>,
     ): Record<string, string> {
-      const EMPTY_OBJ = {};
-
       currentState ??= EMPTY_OBJ;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/debug/profileHooks.ts` around lines 42 - 61, The
EMPTY_OBJ constant is being reallocated on every call to
buildSetStateProfileMarkArgs; hoist a single immutable shared EMPTY_OBJ (e.g.,
const EMPTY_OBJ = {} typed as Record<string, unknown>) out of the function and
remove the local declaration, then keep using currentState ??= EMPTY_OBJ and
nextState ??= EMPTY_OBJ in buildSetStateProfileMarkArgs to avoid per-call
allocations.
🤖 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/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx`:
- Around line 189-191: The test currently sets globalThis.__PROFILE__ = true
unconditionally in afterEach, which can leak state; modify the setup to capture
the original value in beforeEach (store it in a local variable, e.g.,
originalProfile) and then restore that value in afterEach by assigning
globalThis.__PROFILE__ = originalProfile; update the existing
beforeEach/afterEach around the tests in backgroundSnapshot-profile.test.jsx
(reference globalThis.__PROFILE__, beforeEach, afterEach) so the original value
is preserved and restored to avoid cross-test pollution.

In `@packages/react/runtime/src/backgroundSnapshot.ts`:
- Around line 395-510: The tracedPush call sites inside the helper (the
hydration loop in backgroundSnapshot.ts) eagerly construct trace arg objects
(calling getSnapshotVNodeSource and allocating {args: {...}}) even when
shouldProfile is false; change tracedPush usage so trace arguments are created
lazily — either update tracedPush to accept a zero-arg thunk for the traceOption
(e.g., () => ({ args: {...} })) and only invoke it when shouldProfile is true,
or wrap construction of getSnapshotVNodeSource(...) and the args object with if
(shouldProfile) before calling tracedPush; apply this pattern to all tracedPush
call sites in the helper (including the dynamicPartIndex branches and the
extraProps loop) so no trace-related work runs on the non-profile path.

---

Duplicate comments:
In `@packages/react/runtime/src/debug/profileHooks.ts`:
- Around line 159-172: The cleanup currently deletes vnode[sPatchLength] for all
function-type vnodes even when __globalSnapshotPatch is falsy; to fix, move the
delete vnode[sPatchLength] inside the same conditional that calls profileMark
(the if (__globalSnapshotPatch && vnode[sPatchLength] ===
__globalSnapshotPatch.length) block) in the hook callback so sPatchLength is
removed only when the snapshot-patch condition is met (references: sPatchLength,
__globalSnapshotPatch, profileMark, the hook(options, DIFFED, (old, vnode:
PatchedVNode) => { ... }) callback).

---

Nitpick comments:
In `@packages/react/runtime/src/debug/profileHooks.ts`:
- Around line 63-79: The setState hook currently calls old?.call(this, state,
callback) before invoking profileMark, so this[NEXT_STATE] reflects the
post-merge state; add a one-line comment in the setState hook (near the
old?.call(...) and profileMark(...) calls) explaining that the ordering is
intentional to capture the merged/after-state for buildSetStateProfileMarkArgs
and that DIRTY, sFlowID/profileFlowId are used to gate/identify the trace.
Ensure the comment references setState, NEXT_STATE, profileMark, and
buildSetStateProfileMarkArgs so future readers understand why we profile after
calling the original setState.
- Around line 42-61: The EMPTY_OBJ constant is being reallocated on every call
to buildSetStateProfileMarkArgs; hoist a single immutable shared EMPTY_OBJ
(e.g., const EMPTY_OBJ = {} typed as Record<string, unknown>) out of the
function and remove the local declaration, then keep using currentState ??=
EMPTY_OBJ and nextState ??= EMPTY_OBJ in buildSetStateProfileMarkArgs to avoid
per-call allocations.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b108dd8 and 3679f99.

📒 Files selected for processing (5)
  • .changeset/free-dragons-mate.md
  • packages/react/runtime/__test__/debug/backgroundSnapshot-profile.test.jsx
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/debug/profileHooks.ts
  • packages/react/runtime/src/lynx/tt.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • .changeset/free-dragons-mate.md
  • packages/react/runtime/src/lynx/tt.ts

@Yradex
Copy link
Copy Markdown
Collaborator Author

Yradex commented Feb 24, 2026

@upupming Good catch!

  1. Hook Index: You're right, that extra was shifting the IDs. I've switched it to a plain closure so it no longer consumes a hook slot. Fixed the tests as well.
  2. PROFILE: Fair point. Since it can't bypass compiler-time DCE anyway, I've just removed the runtime injection to keep things simple.
  3. Tree-shaking: Added the mark as suggested.

Just pushed the updates, let me know what you think!

@Yradex Yradex force-pushed the wt/render-trace-20260211 branch from b625fca to 627f1c2 Compare February 26, 2026 08:10
@Yradex Yradex marked this pull request as draft February 26, 2026 08:50
@Yradex
Copy link
Copy Markdown
Collaborator Author

Yradex commented Feb 26, 2026

@codex review

@Yradex Yradex marked this pull request as ready for review February 26, 2026 09:40
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a6b4df991

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Collaborator

@upupming upupming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Yradex Yradex merged commit 18cee9e into lynx-family:main Mar 3, 2026
46 of 47 checks passed
@Yradex Yradex deleted the wt/render-trace-20260211 branch March 3, 2026 03:09
colinaaa pushed a commit that referenced this pull request Mar 9, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @lynx-js/devtool-mcp-server@0.5.0

### Minor Changes

- Use `@lynx-js/devtool-connector` instead of
`@lynx-js/debug-router-connector`.
([#2284](#2284))

The new connector avoids using keep-alive connections, which makes the
connection much more reliable.

- **BREAKING CHANGE**: Remove the `./debug-router-connector` exports.
([#2284](#2284))

## @lynx-js/web-elements@0.12.0

### Minor Changes

- feat: add `willchange` event to `x-viewpager-ng`
([#2305](#2305))

### Patch Changes

- fix: firefox `@supports(width:1rex)`
([#2288](#2288))

- fix: check computed overflow style in `getTheMostScrollableKid` to
avoid treating `overflow: visible` elements as scroll containers
([#2309](#2309))

- fix: the inline-truncation should only work as a direct child of
x-text ([#2287](#2287))

- fix: getVisibleCells cannot work in firefox due to
contentvisibilityautostatechange not propagate list-item
([#2308](#2308))

- fix: foldview stuck issue
([#2304](#2304))

## @lynx-js/gesture-runtime@2.1.3

### Patch Changes

- Optimize gesture callbacks and relationships to prevent unnecessary
gesture registration and rerenders.
([#2277](#2277))

## @lynx-js/react@0.116.5

### Patch Changes

- Improve React runtime hook profiling.
([#2235](#2235))
Enable Profiling recording first, then enter the target page so the
trace includes full render/hydrate phases.

- Record trace events for `useEffect` / `useLayoutEffect` hook entry,
callback, and cleanup phases.
    -   Log trace events for `useState` setter calls.
- Wire `profileFlowId` support in debug profile utilities and attach
flow IDs to related hook traces.
- Instrument hydrate/background snapshot profiling around patch
operations with richer args (e.g. snapshot id/type, dynamic part index,
value type, and source when available).
- Capture vnode source mapping in dev and use it in profiling args to
improve trace attribution.
- Expand debug test coverage for profile utilities, hook profiling
behavior, vnode source mapping, and hydrate profiling branches.

- refactor: call loadWorkletRuntime once in each module
([#2315](#2315))

## @lynx-js/rspeedy@0.13.5

### Patch Changes

- feat: opt-in the web platform's new binary output format
([#2281](#2281))

    Introduce a new flag to enable the new binary output format.

Currently it's an internal-use-only flag that will be removed in the
future; set the corresponding environment variable to 'true' to enable
it.

- Avoid generating `Rsbuild vundefined` in greeting message.
([#2275](#2275))

-   Updated dependencies \[]:
    -   @lynx-js/web-rsbuild-server-middleware@0.19.8

## @lynx-js/lynx-bundle-rslib-config@0.2.2

### Patch Changes

- Support bundle and load css in external bundle
([#2143](#2143))

## @lynx-js/external-bundle-rsbuild-plugin@0.0.3

### Patch Changes

- Updated dependencies
\[[`c28b051`](c28b051),
[`4cbf809`](4cbf809)]:
    -   @lynx-js/externals-loading-webpack-plugin@0.0.4

## @lynx-js/react-rsbuild-plugin@0.12.10

### Patch Changes

- Support bundle and load css in external bundle
([#2143](#2143))

- Updated dependencies
\[[`59f2933`](59f2933),
[`453e006`](453e006)]:
    -   @lynx-js/template-webpack-plugin@0.10.5
    -   @lynx-js/css-extract-webpack-plugin@0.7.0
    -   @lynx-js/react-webpack-plugin@0.7.4
    -   @lynx-js/react-alias-rsbuild-plugin@0.12.10
    -   @lynx-js/use-sync-external-store@1.5.0
    -   @lynx-js/react-refresh-webpack-plugin@0.3.4

## @lynx-js/web-core-wasm@0.0.5

### Patch Changes

- Updated dependencies
\[[`4963907`](4963907),
[`8fd936a`](8fd936a),
[`0d41253`](0d41253),
[`d32c4c6`](d32c4c6),
[`7518b72`](7518b72),
[`fca9d4a`](fca9d4a)]:
    -   @lynx-js/web-elements@0.12.0

## @lynx-js/externals-loading-webpack-plugin@0.0.4

### Patch Changes

- perf: optimize external bundle loading by merging multiple
`fetchBundle` calls for the same URL into a single request.
([#2307](#2307))

- Support bundle and load css in external bundle
([#2143](#2143))

## @lynx-js/template-webpack-plugin@0.10.5

### Patch Changes

- feat: allow `templateDebugUrl` to be customized via
`output.publicPath` or the `beforeEncode` hook.
([#2274](#2274))

- feat: opt-in the web platform's new binary output format
([#2281](#2281))

    Introduce a new flag to enable the new binary output format.

Currently it's an internal-use-only flag that will be removed in the
future; set the corresponding environment variable to 'true' to enable
it.

-   Updated dependencies \[]:
    -   @lynx-js/web-core-wasm@0.0.5

## create-rspeedy@0.13.5



## @lynx-js/react-alias-rsbuild-plugin@0.12.10



## upgrade-rspeedy@0.13.5

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants