-
Notifications
You must be signed in to change notification settings - Fork 47.2k
Comparing changes
Open a pull request
base repository: facebook/react
base: 4c58fce7777f2760f4a93091ca4fca0e3fc2f48c
head repository: facebook/react
compare: a03254bc60b06c535c37e43c53b1fd40757b2ef4
- 4 commits
- 13 files changed
- 2 contributors
Commits on Sep 5, 2024
-
[compiler runtime] repro: infinite render with useMemoCache + render …
…phase updates (#30849) Repro for an infinite render bug we found when testing internally. See equivalent codesandbox repro [here](https://codesandbox.io/p/sandbox/epic-euclid-mr7lm3). When render phase updates cause a re-render, useMemoCache arrays for the fiber are [cleared](https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L819) and [recreated on every retry](https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L1223) while hook state is preserved. This pattern (queuing re-renders on the current fiber during render) is perfectly valid. I believe this is a bug as React compiler currently replaces `useMemo`s with `useMemoCache` calls and inlined instructions, taking care to preserve existing memoization dependencies. This should be the identity transform, but runtime implementation differences mean that uncompiled code behaves as expected (no infinite render) while compiled code fails to render.
Configuration menu - View commit details
-
Copy full SHA for d72e477 - Browse repository at this point
Copy the full SHA d72e477View commit details -
[DevTools] Refactor Forcing Fallback / Error of Suspense / Error Boun…
…daries (#30870) First, this basically reverts 1f3892e to use a Map/Set to track what is forced to suspend/error again instead of flags on the Instance. The difference is that now the key in the Fiber itself instead of the ID. Critically this avoids the fiberToFiberInstance map to look up whether or not a Fiber should be forced to suspend when asked by the renderer. This also allows us to force suspend/error on filtered instances. It's a bit unclear what should happen when you try to Suspend or Error a child but its parent boundary is filtered. It was also inconsistent between Suspense and Error due to how they were implemented. I think conceptually you're trying to simulate what would happen if that Component errored or suspended so it would be misleading if we triggered a different boundary than would happen in real life. So I think we should trigger the nearest unfiltered Fiber, not the nearest Instance. The consequence of this however is that if this instance was filtered, there's no way to undo it without refreshing or removing the filter. This is an edge case though since it's unusual you'd filter these in the first place. It used to be that Suspense walked the store in the frontend and Error walked the Fibers in the backend. They also did this somewhat eagerly. This simplifies and unifies the model by passing the id of what you clicked in the frontend and then we walk the Fiber tree from there in the backend to lazily find the boundary. However I also eagerly walk the tree at first to find whether we have any Suspense or Error boundary parents at all so we can hide the buttons if not. This also implements it to work with VirtualInstances using #30865. I find the nearest Fiber Instance downwards filtered or otherwise. Then from its parent we find the nearest Error or Suspense boundary. That's because VirtualInstance will always have their inner Fiber as an Instance but they might not have their parent since it might be filtered. Which would potentially cause us to skip over a filtered parent Suspense boundary.
Configuration menu - View commit details
-
Copy full SHA for a06cd9e - Browse repository at this point
Copy the full SHA a06cd9eView commit details
Commits on Sep 6, 2024
-
[Fiber] Extract Functions that Call User Space and Host Configs in Co…
…mmit to Separate Modules (#30881) This is mostly just moves and same code extracted into utility functions. This is to help clarify what needs to be wrapped in try/catch and runWithFiberInDEV. I'll do the runWithFiberInDEV changes in a follow up. This leaves ReactCommitWork mostly to do matching on the tag and the recursive loops.
Configuration menu - View commit details
-
Copy full SHA for fe03c56 - Browse repository at this point
Copy the full SHA fe03c56View commit details -
[Fiber] Move runWithFiberInDEV from CommitWork to CommitEffects (#30882)
Stacked on #30881. Move `runWithFiberInDEV` from the recursive part of the commit phase and instead wrap each call into user space. These should really map 1:1 with where we're using `try/catch` since that's where we're calling into user space. The goal of this is to avoid the extra stack frames added by `enableOwnerStacks` in the recursive parts to avoid stack overflow. This way we only have a couple of extra at the end of the stack instead of a couple of extra at every depth of the tree.
Configuration menu - View commit details
-
Copy full SHA for a03254b - Browse repository at this point
Copy the full SHA a03254bView commit details
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff 4c58fce7777f2760f4a93091ca4fca0e3fc2f48c...a03254bc60b06c535c37e43c53b1fd40757b2ef4