* Convert useSES shim tests to use React DOM
Idea is that eventually we'll run these tests against an actual build of
React DOM 17 to test backwards compatibility.
* Implement getServerSnapshot in userspace shim
If the DOM is not present, we assume that we are running in a server
environment and return the result of `getServerSnapshot`.
This heuristic doesn't work in React Native, so we'll need to provide
a separate native build (using the `.native` extension). I've left this
for a follow-up.
We can't call `getServerSnapshot` on the client, because in versions of
React before 18, there's no built-in mechanism to detect whether we're
hydrating. To avoid a server mismatch warning, users must account for
this themselves and return the correct value inside `getSnapshot`.
Note that none of this is relevant to the built-in API that is being
added in 18. This only affects the userspace shim that is provided
for backwards compatibility with versions 16 and 17.
Adds a third argument called `getServerSnapshot`.
On the server, React calls this one instead of the normal `getSnapshot`.
We also call it during hydration.
So it represents the snapshot that is used to generate the initial,
server-rendered HTML. The purpose is to avoid server-client mismatches.
What we render during hydration needs to match up exactly with what we
render on the server.
The pattern is for the server to send down a serialized copy of the
store that was used to generate the initial HTML. On the client, React
will call either `getSnapshot` or `getServerSnapshot` on the client as
appropriate, depending on whether it's currently hydrating.
The argument is optional for fully client rendered use cases. If the
user does attempt to omit `getServerSnapshot`, and the hook is called
on the server, React will abort that subtree on the server and
revert to client rendering, up to the nearest Suspense boundary.
For the userspace shim, we will need to use a heuristic (canUseDOM)
to determine whether we are in a server environment. I'll do that in
a follow up.
Until useSyncExternalStore is finalized, the shim should import the
prefixed version (unstable_useSyncExternalStore), which is available
in the experimental builds. That way our early testers can start
using it.
When you pass an unmemoized selector to useSyncExternalStoreExtra, we
have to reevaluate it on every render because we don't know whether
it depends on new props.
However, after reevalutating it, we should still compare the result
to the previous selection with `isEqual` and reuse the old one if it
hasn't changed.
Originally I did not implement this, because if the selector changes due
to new props or state, the component is going to have to re-render
anyway. However, it's still useful to return a memoized selection
when possible, because it may be the input to a downstream memoization.
In the test I wrote, the example I chose is selecting a list of
items from the store, and passing the list as a prop to a memoized
component. If the list prop is memoized, we can bail out.
* [useSyncExternalStore] Remove extra hook object
Because we already track `getSnapshot` and `value` on the store
instance, we don't need to also track them as effect dependencies. And
because the effect doesn't require any clean-up, we don't need to track
a `destroy` function.
So, we don't need to store any additional state for this effect. We can
call `pushEffect` directly, and only during renders where something
has changed.
This saves some memory, but my main motivation is because I plan to use
this same logic to schedule a pre-commit consistency check. (See the
inline comments for more details.)
* Split shouldTimeSlice into two separate functions
Lanes that are blocking (SyncLane, and DefaultLane inside a blocking-
by-default root) are always blocking for a given root. Whereas expired
lanes can expire while the render phase is already in progress.
I want to check if a lane is blocking without checking whether it
expired, so I split `shouldTimeSlice` into two separate functions.
I'll use this in the next step.
* Check for store mutations before commit
When a store is read for the first time, or when `subscribe` or
`getSnapshot` changes, during a concurrent render, we have to check
at the end of the render phase whether the store was mutated by
an concurrent event.
In the userspace shim, we perform this check in a layout effect, and
patch up any inconsistencies by scheduling another render + commit.
However, even though we patch them up in the next render, the parent
layout effects that fire in the original render will still observe an
inconsistent tree.
In the native implementation, we can instead check for inconsistencies
right after the root is completed, before entering the commit phase. If
we do detect a mutaiton, we can discard the tree and re-render before
firing any effects. The re-render is synchronous to block further
concurrent mutations (which is also what we do to recover from tearing
bugs that result in an error). After the synchronous re-render, we can
assume the tree the tree is consistent and continue with the normal
algorithm for finishing a completed root (i.e. either suspend
or commit).
The result is that layout effects will always observe a consistent tree.
Replaced network.onRequestFinished() caching with network.getHAR() so that we can avoid redundantly (pre) caching JavaScript content. In the event that the HAR log doesn't contain a match, we'll fall back to fetching from the Network (and hoping for a cache hit from that layer).
I've tested both internally (internal Facebook DEV server) and externally (Code Sandbox) and it seems like this approach results in cache hits, so long as DevTools is opened when the page loads. (Otherwise it falls back to fetch().)
* update error message to include useLayoutEffect or useEffect on bad effect return
* Update packages/react-reconciler/src/ReactFiberCommitWork.new.js
Co-authored-by: Ricky <rickhanlonii@gmail.com>
* use existing import
Co-authored-by: Ricky <rickhanlonii@gmail.com>
Reverts part of #22275 (adding .catch() to Thenables in Suspense caches) in response to #22275 (comment).
After taking another look with fresh eyes, I think I see the "uncaught error" issue I initially noticed was in checkForUpdate() (which did not pass an error handler to .then)
This commit builds on PR #22260 and makes the following changes:
* Adds a DevTools feature flag for named hooks support. (This allows us to disable it entirely for a build via feature flag.)
* Adds a new Suspense cache for dynamically imported modules. (This allows a component to suspend while importing an external code chunk– like the hook names parsing code).
* DevTools supports a hookNamesModuleLoaderFunction param to import the hook names module. I wish this could be handles as part of the react-devtools-shared package, but I'm not sure how to configure Webpack (4) to serve the chunk from react-devtools-inline. This seemed like a reasonable workaround.
The PR also contains an additional unrelated change:
* Removes pre-fetch optimization (added in DevTools: Improve named hooks network caching #22198). This optimization was mostly only important for cases where sources needed to be re-downloaded, something which we can now avoid in most cases¹ thanks to using cached responses already loaded by the page. (I tested this locally on Facebook and this change has no negative performance impact. There is still some overhead from serializing the JS through the Bridge but that's constant between the two approaches.)
¹ The case where we don't benefit from cached responses is when DevTools are opened after the page has already loaded certain scripts. This seems uncommon enough that I don't think it justified the added complexity of prefetching.
* Add test that triggers infinite update loop
In 18, passive effects are flushed synchronously if they are the
result of a synchronous update. We have a guard for infinite update
loops that occur in the layout phase, but it doesn't currently work for
synchronous updates from a passive effect.
The reason this probably hasn't come up yet is because synchronous
updates inside the passive effect phase are relatively rare: you either
have to imperatively dispatch a discrete event, like `el.focus`, or you
have to call `ReactDOM.flushSync`, which triggers a warning. (In
general, updates inside a passive effect are not encouraged.)
I discovered this because `useSyncExternalStore` does sometimes
trigger updates inside the passive effect phase.
This commit adds a failing test to prove the issue exists. I will fix
it in the next commit.
* Fix failing test added in previous commit
The way we detect a "nested update" is if there's synchronous work
remaining at the end of the commit phase.
Currently this check happens before we synchronously flush the passive
effects. I moved it to after the effects are fired, so that it detects
whether synchronous work was scheduled in that phase.
* Test bad useEffect return value with noop-renderer
* Use previous "root"-approach
Tests should now be invariant under variants
* Add same test for layout effects
* Fix#21972: Add `onResize` event to video elements
This is a simple fix for #21972 that adds support for the `onResize` media event.
I created a separate `videoEventTypes` array since I doubt anyone will want to add `onResize` to an audio event. It would simplify the code a bit to just add `resize` to the `mediaEventTypes` array, if that’s preferred.
Pre-commit checklist ([source](https://reactjs.org/docs/how-to-contribute.html#sending-a-pull-request))
✅ Fork the repository and create your branch from `main`.
✅ Run `yarn` in the repository root.
✅ If you’ve fixed a bug or added code that should be tested, add tests!
✅ Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development.
✅ Run `yarn test --prod` to test in the production environment.
✅ If you need a debugger, run `yarn debug-test --watch TestName`, open chrome://inspect, and press “Inspect”.
✅ Format your code with prettier (`yarn prettier`).
✅ Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files.
✅ Run the Flow typechecks (`yarn flow`).
✅ If you haven’t already, complete the CLA.
* Consolidate `videoEventTypes` array into `mediaEventTypes`
Adds support for useSyncExternalStore to react-debug-tools, which in
turn adds support for React Devtools.
Test plan: I added a test to ReactHooksInspectionIntegration, based on
existing one for useMutableSource.
The userspace shim of useSyncExternalStore uses a useState hook because
it's the only way to trigger a re-render. We don't actually use the
queue to store anything, because we read the current value directly
from the store.
In the native implementation, we can schedule an update on the fiber
directly, without the overhead of a queue.
This adds an initial implementation of useSyncExternalStore to the
fiber reconciler. It's mostly a copy-paste of the userspace
implementation, which is not ideal but is a good enough starting place.
The main change we'll want to make to this native implementation is to
move the tearing checks from the layout phase to an earlier, pre-commit
phase so that code that runs in the commit phase always observes a
consistent tree.
Follow-ups:
- Implement in Fizz
- Implement in old SSR renderer
- Implement in react-debug-hooks
