Fix logComponentRender breaking proxy objects like tRPC client #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…book#35672) Fixes facebook#31463, facebook#30114. When switching between roots in the profiler flamegraph, the commit index was preserved from the previous root. This caused an error "Invalid commit X. There are only Y commits." when the new root had fewer commits than the selected index. This fix resets the commit index to 0 (or null if no commits) when the commitData changes, which happens when switching roots.
facebook#35598) Currently, IO that finished before the request started is not considered IO: https://github.com/facebook/react/blob/6a0ab4d2dd62dfdf8881ba1c3443c6d5acedc871/packages/react-server/src/ReactFlightServer.js#L5338-L5343 This leads to loss of debug info when a flight stream is deserialized and serialized again. We can solve this by allowing "when the the request started" to be set to a point in the past, when the original stream started by doing ```js const startTime = performance.now() + performance.timeOrigin // ... stuff happens and time passes... ReactServer.renderToReadableStream(..., { startTime }) ```
Allows Server Components to import Context from a `"use client'` module and render its Provider. Only tricky part was that I needed to add `REACT_CONTEXT_TYPE` handling in mountLazyComponent so lazy-resolved Context types can be rendered. Previously only functions, REACT_FORWARD_REF_TYPE, and REACT_MEMO_TYPE were handled. Tested in the Flight fixture. ty bb claude Closes facebook#35340 --------- Co-authored-by: Sophie Alpert <git@sophiebits.com>
This is a combination of a) a subagent for investigating compiler errors and b) testing that agent by fixing bugs with for loops within try/catch. My recent diffs to support maybe-throw within value blocks was incomplete and handled many cases, like optionals/logicals/etc within try/catch. However, the handling for for loops was making more assumptions and needed additional fixes. Key changes: * `maybe-throw` terminal `handler` is now nullable. PruneMaybeThrows nulls the handler for blocks that cannot throw, rather than changing to a `goto`. This preserves more information, and makes it easier for BuildReactiveFunction's visitValueBlock() to reconstruct the value blocks * Updates BuildReactiveFunction's handling of `for` init/test/update (and similar for `for..of` and `for..in`) to correctly extract value blocks. The previous logic made assumptions about the shape of the SequenceExpression which were incorrect in some cases within try/catch. The new helper extracts a flattened SequenceExpression. Supporting changes: * The agent itself (tested via this diff) * Updated the script for invoking snap to keep `compiler/` as the working directory, allowing relative paths to work more easily * Add an `--update` (`-u`) flag to `yarn snap minimize`, which updates the fixture in place w the minimized version
facebook#35688) More snap improvements for use with agents: * `yarn snap compile [--debug] <path>` for compiling any file, optionally with debug logs * `yarn snap minimize <path>` now accepts path as a positional param for consistency w 'compile' command * Both compile/minimize commands properly handle paths relative to the compiler/ directory. When using `yarn snap` the current working directory is compiler/packages/snap, but you're generally running it from the compiler directory so this matches expectations of callers better.
When React's performance tracking code tries to diff props for logging, it accesses properties on objects. For proxy objects like tRPC clients, this can trigger proxy traps that throw errors, breaking the application. This fix wraps property access in try-catch blocks in both addObjectToProperties and addObjectDiffToProperties functions. When accessing a property throws an error, we simply skip that property and continue with the rest. Fixes facebook#35657 Co-Authored-By: luigisambuy@gmail.com <luigisambuy@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #35657 - React's performance tracking code was breaking applications that use proxy-based libraries like tRPC client.
When React's
logComponentRendertries to diff props for logging in DEV mode, it accesses properties on objects usingobject[key]. For proxy objects like tRPC clients, this triggers proxy traps that can throw errors (e.g.,client[procedureType] is not a function), breaking the application.This fix wraps property access in try-catch blocks in two functions:
addObjectToProperties- when iterating over object properties for loggingaddObjectDiffToProperties- when comparing prev/next props for diff loggingWhen accessing a property throws an error, we skip that property and continue with the rest.
Review & Testing Checklist for Human
Test Plan
client[procedureType] is not a functionerror no longer occursNotes