feat(react): support profile in production build #1336
Conversation
🦋 Changeset detectedLatest commit: 45f2597 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis change introduces advanced profiling support to the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR enables profiling capabilities in production builds by adding dynamic profiling support that can be enabled at runtime, moving away from build-time-only profiling. The key changes refactor the profiling system to use runtime API calls instead of compile-time flags and console methods.
- Adds dynamic profiling support through
lynx.performanceAPI methods - Refactors profiling hooks to use a new utility function for cleaner code
- Updates tests to use the new profiling API instead of console methods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/runtime/src/utils.ts | Adds new hook utility function for monkey-patching objects |
| packages/react/runtime/src/lynx.ts | Updates profiling condition to check runtime state in addition to build flag |
| packages/react/runtime/src/debug/profile.ts | Refactors profiling implementation to use lynx.performance API and hook utility |
| packages/react/runtime/test/utils/globals.js | Adds mock profiling methods and test validation for balanced profile calls |
| packages/react/runtime/test/debug/profile.test.jsx | Updates test expectations to use new profiling API |
| .changeset/forty-aliens-play.md | Documents the feature addition |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1336 will not alter performanceComparing Summary
|
React Example#3946 Bundle Size — 236.85KiB (+0.67%).45f2597(current) vs ea325d6 main#3934(baseline) Bundle metrics
|
| Current #3946 |
Baseline #3934 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
38.04% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
158(-0.63%) |
159 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.85%(+0.09%) |
45.81% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 regression
| Current #3946 |
Baseline #3934 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.09KiB (+1.77%) |
89.5KiB |
Bundle analysis report Branch hzy:p/hzy/profile_in_production_... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#3942 Bundle Size — 344.07KiB (+0.08%).45f2597(current) vs ea325d6 main#3930(baseline) Bundle metrics
|
| Current #3942 |
Baseline #3930 |
|
|---|---|---|
| Initial JS |
143.27KiB |
143.27KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
| Cache Invalidation |
17.58% |
41.63% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
| Modules |
211(-0.47%) |
212 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.96% |
3.96% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #3942 |
Baseline #3930 |
|
|---|---|---|
| JS |
229.27KiB (+0.12%) |
229.01KiB |
| Other |
82.95KiB |
82.95KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch hzy:p/hzy/profile_in_production_... Project dashboard
Generated by RelativeCI Documentation Report issue
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
868c44f to
b219b4b
Compare
![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
♻️ Duplicate comments (2)
packages/react/runtime/src/debug/profile.ts (1)
10-21: Check forprofileEndavailability before using itThe code checks for
profileStartbut usesprofileEndwithout verification. This could cause runtime errors ifprofileEndis undefined.Apply this fix to ensure both methods are available:
export function initProfileHook(): void { // `profileStart`, `profileEnd`, `profileMark` and `profileFlowId` are only available in Lynx 3.0+ // if (!isSdkVersionGt(3, 0)) { - if (!lynx.performance || typeof lynx.performance.profileStart !== 'function') { + if (!lynx.performance || typeof lynx.performance.profileStart !== 'function' || typeof lynx.performance.profileEnd !== 'function') { return; }packages/react/runtime/src/utils.ts (1)
71-81: Use proper type parameter for arguments to maintain type safetyThe function signature uses
unknown[]for args but the type definition expects specific parameter types.Consider using
Ptype parameter:- object[key] = function(this: T, ...args: unknown[]) { + object[key] = function(this: T, ...args: P) { return fn.call(this, oldFn, ...args); } as T[K];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(1 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lynx.ts(1 hunks)packages/react/runtime/src/utils.ts(1 hunks)
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/debug/profile.ts (3)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
packages/react/runtime/src/utils.ts (1)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/react/runtime/src/lynx.ts
- .changeset/forty-aliens-play.md
- packages/react/runtime/test/debug/profile.test.jsx
- packages/react/runtime/test/utils/globals.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/debug/profile.ts (3)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
packages/react/runtime/src/utils.ts (1)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
b219b4b to
f3d5d0a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/react/runtime/src/debug/profile.ts (1)
13-15: Add missing method availability checksThe availability check only verifies
profileStartandprofileEnd, but the code later usesprofileMark(line 49) andprofileFlowId(line 50) without verification, which could cause runtime errors.Add these methods to the availability check:
- if (!lynx.performance || typeof lynx.performance.profileStart !== 'function') { + if (!lynx.performance || + typeof lynx.performance.profileStart !== 'function' || + typeof lynx.performance.profileEnd !== 'function' || + typeof lynx.performance.profileMark !== 'function' || + typeof lynx.performance.profileFlowId !== 'function') { return; }
🧹 Nitpick comments (1)
packages/react/runtime/src/debug/profile.ts (1)
79-79: Fix typo in constant nameThere's a typo in the constant name.
- const EMPYT_OBJ = {}; + const EMPTY_OBJ = {};And update the usage:
- currentState ??= EMPYT_OBJ; - nextState ??= EMPYT_OBJ; + currentState ??= EMPTY_OBJ; + nextState ??= EMPTY_OBJ;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(1 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lynx.ts(1 hunks)packages/react/runtime/src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/test/debug/profile.test.jsx
- .changeset/forty-aliens-play.md
- packages/react/runtime/test/utils/globals.js
- packages/react/runtime/src/utils.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/debug/profile.ts (3)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
🔇 Additional comments (5)
packages/react/runtime/src/debug/profile.ts (5)
4-8: LGTM: Clean importsThe imports are well-organized and all utilities are properly used throughout the file.
22-40: LGTM: Well-structured diff profiling hooksThe implementation correctly:
- Filters for function components only
- Handles flow ID lifecycle with proper cleanup
- Maintains balanced profileStart/profileEnd calls
- Preserves existing hook chains
42-55: LGTM: Sophisticated setState profilingThe implementation intelligently:
- Only profiles when the component is dirty (actual state change)
- Manages flow IDs efficiently with the WeakMap
- Generates detailed profiling metadata about state changes
- Properly chains the original setState call
The conditional profiling based on
DIRTYflag is a good optimization.
58-73: LGTM: Clever render method profilingThe temporary method replacement pattern is well-implemented:
- Ensures balanced profiling calls with try/finally
- Properly restores the original render method
- Maintains the correct component context
75-94: LGTM: Comprehensive state change metadataThe helper function provides valuable profiling information:
- Current and next state keys for debugging
- Shallow diff to identify what actually changed
- Proper null/undefined handling
- JSON serialization for readability
The logic correctly assumes
nextStateis a superset ofcurrentStatefor the diff calculation.
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
f3d5d0a to
140a049
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react/runtime/src/debug/profile.ts (1)
84-84: Fix typo in constant name.The constant name has a typo:
EMPYT_OBJshould beEMPTY_OBJ.- const EMPYT_OBJ = {}; + const EMPTY_OBJ = {};Also update the usage on lines 86-87:
- currentState ??= EMPYT_OBJ; - nextState ??= EMPYT_OBJ; + currentState ??= EMPTY_OBJ; + nextState ??= EMPTY_OBJ;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(1 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lynx.ts(1 hunks)packages/react/runtime/src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/test/debug/profile.test.jsx
- packages/react/runtime/test/utils/globals.js
- .changeset/forty-aliens-play.md
- packages/react/runtime/src/utils.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/debug/profile.ts (3)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
🔇 Additional comments (7)
packages/react/runtime/src/debug/profile.ts (7)
4-8: LGTM! Import statements properly support the refactored profiling functionality.The imports correctly include all necessary utilities and constants for the new Lynx profiling API integration.
11-20: Good defensive programming with comprehensive API availability checks.The early exit logic now properly verifies all required profiling methods (
profileStart,profileEnd,profileMark,profileFlowId) are available before proceeding. This addresses the previous review feedback about missing checks.
22-25: Well-designed flow ID management setup.Using
WeakMapfor component-to-flow-ID association is appropriate as it enables automatic cleanup when components are garbage collected. Method binding ensures correct execution context.
27-45: Well-implemented DIFF/DIFFED profiling hooks.The logic correctly:
- Profiles only function components
- Uses
mapGetAndDeletefor efficient flow ID management- Properly starts/ends profiling traces with meaningful labels
- Maintains backward compatibility by calling old hooks
47-60: Sophisticated setState profiling implementation with efficient dirty checking.The hook correctly:
- Only profiles when
this[DIRTY]is true for performance efficiency- Uses
nullishCoalescingMapSetfor optimal flow ID management- Provides detailed state change information via the helper function
- Maintains proper method call order
63-78: Robust RENDER hook with proper error handling and cleanup.The implementation uses a sound pattern:
- Temporarily wraps the render method with profiling
- Uses try/finally to guarantee cleanup and profiling end
- Restores original render method even if exceptions occur
- Provides meaningful profiling labels with component display names
86-98: Well-designed state change profiling helper with comprehensive information.The function provides valuable profiling data:
- Safe handling of undefined states with nullish coalescing
- Comprehensive state change information (current, next, changed keys)
- Efficient shallow comparison for change detection
- Clear comments explaining the setState behavior assumptions
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
140a049 to
b37d007
Compare
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
2 times, most recently
from
9332ae0 to
8b9dc6d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(1 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lynx.ts(1 hunks)packages/react/runtime/src/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/react/runtime/test/debug/profile.test.jsx
- .changeset/forty-aliens-play.md
- packages/react/runtime/test/utils/globals.js
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/src/utils.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/debug/profile.ts (2)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
🪛 GitHub Check: test-typos
packages/react/runtime/src/debug/profile.ts
[warning] 111-111:
"EMPYT" should be "EMPTY".
[warning] 110-110:
"EMPYT" should be "EMPTY".
[warning] 108-108:
"EMPYT" should be "EMPTY".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: code-style-check
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: CodeQL Analyze (actions)
🔇 Additional comments (7)
packages/react/runtime/src/debug/profile.ts (7)
4-9: LGTM! Import statements are well-organized.The imports correctly support the refactored profiling implementation using the new utility functions and constants.
12-21: Excellent! Comprehensive availability checks address previous concerns.The early exit pattern with checks for all required profiling methods (
profileStart,profileEnd,profileMark,profileFlowId) properly addresses the runtime safety concerns raised in previous reviews.
23-28: Good use of WeakMaps and method binding.Using WeakMaps for component-specific tracking prevents memory leaks, and binding the profiling methods ensures proper context. This is a solid architectural approach.
29-48: Well-structured DIFF hook implementation.The refactored hook correctly uses the new
hookutility, manages flow IDs for tracing, and includes proper conditional logic for background snapshot patching. The component type checking and profiling trace naming are appropriate.
50-66: Excellent DIFFED hook with no-op detection.The implementation properly ends profiling traces, includes smart no-op detection for background processing, and maintains proper cleanup of the snapshot patch length map. The profiling marks will be helpful for debugging performance issues.
68-84: Solid setState hook with proper flow ID management.The implementation correctly wraps
setStatewith profiling, includes proper TypeScript typing, and uses theDIRTYflag optimization to avoid unnecessary profiling. The flow ID management and detailed state change tracking will be valuable for debugging.
87-102: Robust RENDER hook with proper cleanup.The temporary method replacement approach with try/finally ensures accurate profiling of render logic while maintaining proper cleanup. The error handling and context preservation are well-implemented.
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
2 times, most recently
from
5182334 to
e4c6f13
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lynx.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/src/debug/profile.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/runtime/src/lifecycle/patch/commit.ts (1)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
🔇 Additional comments (5)
packages/react/runtime/src/lifecycle/patch/updateMainThread.ts (1)
27-34: LGTM! Conditional profiling implementation looks good.The profiling logic correctly extracts
flowIdsfrom patch options and conditionally starts profiling with appropriate metadata. The@ts-expect-errorcomment acknowledges the type limitation forflowIdsproperty.packages/react/runtime/src/lifecycle/patch/commit.ts (4)
29-29: Good encapsulation with atomic state management.The
takeGlobalFlushOptionsfunction provides atomic access to global flush options with proper cleanup, improving state management compared to direct access patterns.Also applies to: 36-40
72-84: Well-designed type refinement and state management.The
GlobalPatchOptionstype appropriately omits thereloadVersionfield that's managed internally, andtakeGlobalPatchOptionsfollows the same atomic pattern as the flush options helper.
90-166: Excellent refactoring using the hook utility pattern.The commit hook replacement is well-structured using the
hookutility, maintaining all existing functionality while improving encapsulation. The logic flow is preserved: skipping main thread commits, collecting patches, managing background instance cleanup, and coordinating with the native layer.
172-172: Type signature improvement aligns with new type system.Using
GlobalPatchOptionsinstead of the inlineOmittype improves readability and maintainability.
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
6 times, most recently
from
058b4f0 to
22c2457
Compare
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
6409e7c to
0895f4d
Compare
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
7c5b84f to
99d7ad0
Compare
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
99d7ad0 to
2591d46
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts (1)
8-16: Return type missing newly added properties; will trigger excess property errorsYou added
profileMark,profileFlowId, andisProfileRecordingto the returned object, but thePick<NativeApp, ...>return type doesn’t include them. With object-literal excess property checks, this will fail type-checking. Include the new keys in thePick.Apply this diff:
export function createPerformanceApis(timingSystem: TimingSystem): Pick< NativeApp, | 'generatePipelineOptions' | 'onPipelineStart' | 'markPipelineTiming' | 'bindPipelineIdWithTimingFlag' | 'profileStart' | 'profileEnd' + | 'profileMark' + | 'profileFlowId' + | 'isProfileRecording' > {
🧹 Nitpick comments (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts (1)
51-57: Use warn-level logging or add a tracking link (optional)Console errors in production can be noisy for NYI stubs. Consider
console.warnand add a TODO with a concrete issue link in lynx-core for traceability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/hook.js(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(2 hunks)packages/react/runtime/__test__/lifecycle/reload.test.jsx(7 hunks)packages/react/runtime/__test__/lifecycle/updateData.test.jsx(8 hunks)packages/react/runtime/__test__/lynx/timing.test.jsx(6 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lynx.ts(2 hunks)packages/react/runtime/src/renderToOpcodes/index.ts(4 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/react/runtime/test/debug/hook.js
- packages/web-platform/web-constants/src/types/NativeApp.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/react/runtime/test/debug/profile.test.jsx
- packages/react/runtime/test/lifecycle/updateData.test.jsx
- packages/react/runtime/test/lifecycle/reload.test.jsx
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/src/lifecycle/patch/updateMainThread.ts
- packages/react/runtime/test/lynx/timing.test.jsx
- .changeset/forty-aliens-play.md
- packages/react/runtime/test/utils/globals.js
- packages/react/runtime/src/debug/profile.ts
- packages/react/runtime/src/renderToOpcodes/index.ts
- packages/react/runtime/src/lifecycle/patch/commit.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
...eb-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Qingyu Wang <[email protected]> Signed-off-by: Zhiyuan Hong <[email protected]>
Co-authored-by: Qingyu Wang <[email protected]> Signed-off-by: Zhiyuan Hong <[email protected]>
Co-authored-by: Qingyu Wang <[email protected]> Signed-off-by: Zhiyuan Hong <[email protected]>
hzy
force-pushed
the
p/hzy/profile_in_production_build
branch
from
2591d46 to
45f2597
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react/runtime/src/debug/profile.ts (1)
42-61: Avoid double–stringifying state-key arrays
buildSetStateProfileMarkArgsconverts arrays to JSON strings.
If the downstream consumer ofprofileMarklater serialises its arguments again
(the common case for bridge or log forwarding) you end up with escaped JSON
inside JSON.Prefer passing the raw arrays (or a comma-joined string) and let the
transport layer stringify once.- return { - 'current state keys': JSON.stringify(Object.keys(currentState)), - 'next state keys': JSON.stringify(Object.keys(nextState)), - 'changed (shallow diff) state keys': JSON.stringify( - Object.keys(nextState).filter(key => currentState[key] !== nextState[key]), - ), - }; + const currentKeys = Object.keys(currentState); + const nextKeys = Object.keys(nextState); + return { + currentKeys, + nextKeys, + changedKeys: nextKeys.filter(k => currentState[k] !== nextState[k]), + };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/forty-aliens-play.md(1 hunks)packages/react/runtime/__test__/debug/hook.js(1 hunks)packages/react/runtime/__test__/debug/profile.test.jsx(2 hunks)packages/react/runtime/__test__/lifecycle/reload.test.jsx(7 hunks)packages/react/runtime/__test__/lifecycle/updateData.test.jsx(8 hunks)packages/react/runtime/__test__/lynx/timing.test.jsx(6 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/debug/profile.ts(1 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lynx.ts(2 hunks)packages/react/runtime/src/renderToOpcodes/index.ts(4 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/react/runtime/test/debug/hook.js
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/test/lifecycle/reload.test.jsx
- .changeset/forty-aliens-play.md
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/react/runtime/src/lifecycle/patch/updateMainThread.ts
- packages/react/runtime/test/debug/profile.test.jsx
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createPerformanceApis.ts
- packages/react/runtime/test/lifecycle/updateData.test.jsx
- packages/react/runtime/src/renderToOpcodes/index.ts
- packages/react/runtime/src/lifecycle/patch/commit.ts
- packages/react/runtime/test/utils/globals.js
- packages/react/runtime/test/lynx/timing.test.jsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
📚 Learning: 2025-07-18T04:27:18.291Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Applied to files:
packages/react/runtime/src/debug/profile.ts
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/react/runtime/src/debug/profile.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: CodeQL Analyze (actions)
🔇 Additional comments (2)
packages/react/runtime/src/debug/profile.ts (2)
14-27: Guard against multiple initialisations
initProfileHook()can be imported from multiple bundles; without
idempotence it will stack multiplehooklayers around the same methods.Add a static guard:
export function initProfileHook(): void { + if ((initProfileHook as any)._didInit) return; + (initProfileHook as any)._didInit = true;
85-96:TraceOptiontype-safety check
profileOptionsis declaredTraceOption = {}and later assigned
flowId/flowIds. Ensure these fields exist onTraceOption; otherwise
this file will only compile due to--suppressImplicitAnyIndexErrors.If they are not part of the public type, extend locally:
type ExtendedTraceOption = TraceOption & { flowId?: number; flowIds?: number[] }; const profileOptions: ExtendedTraceOption = {};
<!-- Thank you for submitting a pull request! We appreciate the time and effort you have invested in making these changes. Please ensure that you provide enough information to allow others to review your pull request. Upon submission, your pull request will be automatically assigned with reviewers. If you want to learn more about contributing to this project, please visit: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md. --> <!-- The AI summary below will be auto-generated - feel free to replace it with your own. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for advanced profiling in production builds, including new metrics for component diffing, rendering, and state updates. * Enhanced profiling labels for improved traceability, with suggestions for better readability in minified builds. * Introduced profiling instrumentation around patch application and commit phases with flow ID tracking. * **Bug Fixes** * Improved synchronization and accuracy of profiling start and end events to ensure reliable performance data. * **Refactor** * Streamlined profiling logic and lifecycle hook management for better maintainability and encapsulation. * Unified commit hook replacement and improved global options state management. * **Tests** * Updated and expanded test coverage to validate new profiling features and ensure accurate flow ID tracking in patch operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist <!--- Check and mark with an "x" --> - [ ] 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). --------- Signed-off-by: Zhiyuan Hong <[email protected]> Co-authored-by: Qingyu Wang <[email protected]>
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/[email protected] ### Patch Changes - Supports `recyclable` attribute in `<list-item>` to control whether the list item is recyclable. The `recyclable` attribute depends on Lynx Engine 3.4 or later. ([#1388](#1388)) ```jsx <list-item recyclable={false} /> ``` - feat: Support using a host element as direct child of Suspense ([#1455](#1455)) - Add profile in production build: ([#1336](#1336)) 1. `diff:__COMPONENT_NAME__`: how long ReactLynx diff took. 2. `render:__COMPONENT_NAME__`: how long your render function took. 3. `setState`: an instant trace event, indicate when your setState was called. NOTE: `__COMPONENT_NAME__` may be unreadable when minified, setting `displayName` may help. - Add `onBackgroundSnapshotInstanceUpdateId` event on dev for Preact Devtools to keep the correct snapshotInstanceId info. ([#1173](#1173)) - fix: Prevent error when spreading component props onto an element ([#1459](#1459)) - fix: Correctly check for the existence of background functions in MTS ([#1416](#1416)) ```ts function handleTap() { "main thread"; // The following check always returned false before this fix if (myHandleTap) { runOnBackground(myHandleTap)(); } } ``` ## @lynx-js/[email protected] ### Patch Changes - Remove the experimental `provider` option. ([#1432](#1432)) - Add `output.filename.wasm` and `output.filename.assets` options. ([#1449](#1449)) - fix deno compatibility ([#1412](#1412)) - Should call the `api.onCloseBuild` hook after the build finished. ([#1446](#1446)) - Bump Rsbuild v1.4.15. ([#1423](#1423)) - Support using function in `output.filename.*`. ([#1449](#1449)) ## [email protected] ### Patch Changes - Support ESLint for ReactLynx templates ([#1274](#1274)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`c8ce6aa`](c8ce6aa)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the `Package subpath './compat' is not defined by "exports"` error. ([#1460](#1460)) ## @lynx-js/[email protected] ### Patch Changes - Fix `GlobalEventEmitter` type definition, the `emit(eventName: string, data: unknown)` function should recevie an array typed `data` and pass as param list of listeners. ([#1479](#1479)) ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - fix: when `onNativeModulesCall` is delayed in mounting, the NativeModules execution result may be undefined. ([#1457](#1457)) - fix: `onNativeModulesCall` && `onNapiModulesCall` use getter to get. ([#1466](#1466)) - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. ## @lynx-js/[email protected] ### Patch Changes - feat: add autocomplete attribute support for x-input component ([#1444](#1444)) Implements autocomplete attribute forwarding from the x-input custom element to the internal HTML input element in the shadow DOM. This enables standard browser autocomplete functionality for x-input elements. - Add referrerpolicy attribute support to x-image web component ([#1420](#1420)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
4 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Checklist