forked from facebook/react
-
Notifications
You must be signed in to change notification settings - Fork 0
[pull] main from facebook:main #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
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
As part of the new inference model we updated to (correctly) treat
destructuring spread as creating a new mutable object. This had the
unfortunate side-effect of reducing precision on destructuring of props,
though:
```js
function Component({x, ...rest}) {
const z = rest.z;
identity(z);
return <Stringify x={x} z={z} />;
}
```
Memoized as the following, where we don't realize that `z` is actually
frozen:
```js
function Component(t0) {
const $ = _c(6);
let x;
let z;
if ($[0] !== t0) {
const { x: t1, ...rest } = t0;
x = t1;
z = rest.z;
identity(z);
...
```
#34341 was our first thought of how to do this (thanks @poteto for
exploring this idea!). But during review it became clear that it was a
bit more complicated than I had thought. So this PR explores a more
conservative alternative. The idea is:
* Track known sources of frozen values: component props, hook params,
and hook return values.
* Find all object spreads where the rvalue is a known frozen value.
* Look at how such objects are used, and if they are only used to access
properties (PropertyLoad/Destructure), pass to hooks, or pass to jsx
then we can be very confident the object is not mutated. We consider any
such objects to be frozen, even though technically spread creates a new
object.
See new fixtures for more examples.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34900).
* __->__ #34900
* #34887
…#34849) Using `renderToReadableStream` in Node.js with binary data from `fs.readFileSync` (or `Buffer.allocUnsafe`) could cause downstream consumers (like compression middleware) to fail with "Cannot perform Construct on a detached ArrayBuffer". The issue occurs because Node.js uses an 8192-byte Buffer pool for small allocations (< 4KB). When React's `VIEW_SIZE` was 2KB, files between ~2KB and 4KB would be passed through as views of pooled buffers rather than copied into `currentView`. ByteStreams (`type: 'bytes'`) detach ArrayBuffers during transfer, which corrupts the shared Buffer pool and causes subsequent Buffer operations to fail. Increasing `VIEW_SIZE` from 2KB to 4KB ensures all chunks smaller than 4KB are copied into `currentView` (which uses a dedicated 4KB buffer outside the pool), while chunks 4KB or larger don't use the pool anyway. Thus no pooled buffers are ever exposed to ByteStream detachment. This adds 2KB memory per active stream, copies chunks in the 2-4KB range instead of passing them as views (small CPU cost), and buffers up to 2KB more data before flushing. However, it avoids duplicating large binary data (which copying everything would require, like the Edge entry point currently does in `typedArrayToBinaryChunk`). Related issues: - vercel/next.js#84753 - vercel/next.js#84858
…scheme (#34880) <img width="1011" height="811" alt="Screenshot 2025-10-16 at 2 20 46 PM" src="https://github.com/user-attachments/assets/6dea3962-d369-4823-b44f-2c62b566c8f1" /> The selection is now clearer with a wider outline which spans the bounding box if there are multi rects. The color now gets darked changes on hover with a slight animation. The colors are now mixed from constants defined which are consistently used in the rects, the time span in the "suspended by" side bar and the scrubber. I also have constants defined for "server" and "other" debug environments which will be used in a follow up.
#34894) It looks weird when the row is blank when there's no short description for the entry in a group. <img width="328" height="436" alt="Screenshot 2025-10-17 at 12 25 30 AM" src="https://github.com/user-attachments/assets/12f5c55f-a37f-4b6d-913e-f763cec6b211" />
… hovered (#34881) Stacked on #34880. In #34861 I removed the highlight of the real view when hovering the timeline since it was disruptive to stepping through the visuals. This makes it so that when we hover the timeline we highlight the rect with the subtle hover effect added in #34880. We can now just use the one shared state for this and don't need the CSS psuedo-selectors. <img width="603" height="813" alt="Screenshot 2025-10-16 at 3 11 17 PM" src="https://github.com/user-attachments/assets/a018b5ce-dd4d-4e77-ad47-b4ea068f1976" />
…nders (#34885) Stacked on #34881. We don't paint suspense boundaries if there are no suspenders. This does the same with the root. The root is still selectable so you can confirm but there's no affordance drawing attention to click the root. This could happen if you don't use the built-ins of React to load things like scripts and css. It would never happen in something like Next.js where code and CSS is loaded through React-native like RSC. However, it could also happen in the Activity scoped case when all resources are always loaded early.
Stacked on #34885. This refactors the timeline to store not just an id but a complex object for each step. This will later represent a group of boundaries. Each timeline step is assigned an environment name. We pick the last environment name (assumed to have resolved last) from the union of the parent and child environment names. I.e. a child step is considered to be blocked by the parent so if a child isn't blocked on any environment name it still gets marked as the parent's environment name. In a follow up, I'd like to reorder the document order timeline based on environment names to favor loading everything in one environment before the next.
…34893) Stacked on #34892. In the timeline scrubber each timeline entry gets a label and color assigned based on the environment computed for that step. In the rects, we find the timeline step that this boundary is part of and use that environment to assign a color. This is slightly different than picking from the boundary itself since it takes into account parent boundaries. In the "suspended by" section we color each entry individually based on the environment that spawned the I/O. <img width="790" height="813" alt="Screenshot 2025-10-17 at 12 18 56 AM" src="https://github.com/user-attachments/assets/c902b1fb-0992-4e24-8e94-a97ca8507551" />
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )