double invoke test #1

kassens · 2022-11-17T20:30:37Z

[react devtools] Device storage support ([react devtools] Device storage support facebook/react#25452)
[Float] fix coordination of resource identity and hydration ([Float] fix coordination of resource identity and hydration facebook/react#25569)
[Float] handle noscript context for Resources ([Float] handle noscript context for Resources facebook/react#25559)
Try assigning fetch to globalThis if global assignment fails (Try assigning fetch to globalThis if global assignment fails facebook/react#25571)
Remove old react-fetch, react-fs and react-pg libraries (Remove old react-fetch, react-fs and react-pg libraries facebook/react#25577)
Detect and warn if use(promise) is wrapped with try/catch block (Detect and warn if use(promise) is wrapped with try/catch block facebook/react#25543)
Strict Mode: Reuse memoized result from first pass (Strict Mode: Reuse memoized result from first pass facebook/react#25583)
In work loop, add enum of reasons for suspending
Split suspended work loop logic into separate functions
Extract logic for detecting bad fallback to helper
Clean up vestige of useOpaqueIdentifier (Clean up vestige of useOpaqueIdentifier facebook/react#25587)
Fix type check for null (Fix type check for null facebook/react#25595)
Do not unmount layout effects on initial Offscreen mount (Do not unmount layout effects on initial Offscreen mount facebook/react#25592)
[ServerRenderer] Setup for adding data attributes streaming format ([ServerRenderer] Setup for adding data attributes streaming format facebook/react#25567)
Allow uncached IO to stablize (Allow uncached IO to stablize facebook/react#25561)
[Float] handle resource Resource creation inside svg context ([Float] handle resource Resource creation inside svg context facebook/react#25599)
Make host context use null as empty and only error in dev (Make host context use null as empty and only error in dev facebook/react#25609)
refactor isHostResourceType to not receive the context from reconciler and not leak types (refactor isHostResourceType to not receive the context from reconciler and not leak types facebook/react#25610)
Unwrap sync resolved thenables without suspending (Unwrap sync resolved thenables without suspending facebook/react#25615)
[ServerRenderer] Move fizz external runtime implementation to react-dom-bindings ([ServerRenderer] Move fizz external runtime implementation to react-dom-bindings facebook/react#25617)
Remove check in renderDidSuspendDelayIfPossible (Remove check in renderDidSuspendDelayIfPossible facebook/react#25630)
Do not unmount layout effects if ancestor Offscreen is hidden (Do not unmount layout effects if ancestor Offscreen is hidden facebook/react#25628)
[DevTools] add support for HostSingleton & HostResource ([DevTools] add support for HostSingleton & HostResource facebook/react#25616)
Fix useSyncExternalStore dropped update when state is dispatched in render phase (Fix useSyncExternalStore dropped update when state is dispatched in render phase facebook/react#25578)
Don't disappear layout effects unnecessarily (Don't disappear layout effects unnecessarily facebook/react#25660)
[DevTools] bug fix for Hydrating fibers ([DevTools] bug fix for Hydrating fibers facebook/react#25663)
Remove recoverable error when a sync update flows into a dehydrated boundary (Remove recoverable error when a sync update flows into a dehydrated boundary facebook/react#25692)
[react-float] feature detect getRootNode ([react-float] feature detect getRootNode facebook/react#25689)
Bump loader-utils from 2.0.0 to 2.0.4 in /fixtures/flight (Bump loader-utils from 2.0.0 to 2.0.4 in /fixtures/flight facebook/react#25694)
Bump loader-utils from 1.4.0 to 1.4.2 in /fixtures/fizz (Bump loader-utils from 1.4.0 to 1.4.2 in /fixtures/fizz facebook/react#25680)
Eagerly initialize an mutable object for instance.refs (Eagerly initialize an mutable object for instance.refs facebook/react#25696)
Turn on key spread warning in jsx-runtime for everyone (Turn on key spread warning in jsx-runtime for everyone facebook/react#25697)
Turn on string ref deprecation warning for everybody (not codemoddable) (Turn on string ref deprecation warning for everybody (not codemoddable) facebook/react#25383)
Enable warning for defaultProps on function components for everyone (Enable warning for defaultProps on function components for everyone facebook/react#25699)
Force unwind work loop during selective hydration (Force unwind work loop during selective hydration facebook/react#25695)
use: Don't suspend if there are pending updates
Don't reset work loop until stack is unwound
Pass ThenableState to replaySuspendedUnitOfWork
Allow more hooks to be added when replaying mount
Reuse hooks when replaying a suspended component
Check thenable instead of thenableState
Track ThenableState alongside other hooks
Add SyncHydrationLane (Add SyncHydrationLane facebook/react#25698)
Test for double invoke effect of useLayoutEffect

# Summary * This PR adds support for persisting certain settings to device storage, allowing e.g. RN apps to properly patch the console when restarted. * The device storage APIs have signature `getConsolePatchSettings()` and `setConsolePatchSettings(string)`, in iOS, are thin wrappers around the `Library/Settings` turbomodule, and wrap a new TM that uses the `SharedPreferences` class in Android. * Pass device storage getters/setters from RN to DevTools' `connectToDevtools`. The setters are then used to populate values on `window`. Later, the console is patched using these values. * If we receive a notification from DevTools that the console patching fields have been updated, we write values back to local storage. * See facebook/react-native#34903 # How did you test this change? Manual testing, `yarn run test-build-devtools`, `yarn run prettier`, `yarn run flow dom` ## Manual testing setup: ### React DevTools Frontend * Get the DevTools frontend in flipper: * `nvm install -g react-devtools-core`, then replace that package with a symlink to the local package * enable "use globally installed devtools" in flipper * yarn run start in react-devtools, etc. as well ### React DevTools Backend * `yarn run build:backend` in react-devtools-core, then copy-paste that file to the expo app's node_modules directory ### React Native * A local version of React Native can be patched in by modifying an expo app's package.json, as in `"react-native": "rbalicki2/react-native#branch-name"` # Versioning safety * There are three versioned modules to worry about: react native, the devtools frontend and the devtools backend. * The react devtools backend checks for whether a `cachedSettingsStore` is passed from react native. If not (e.g. if React Native is outdated), then no behavior changes. * The devtools backend reads the patched console values from the cached settings store. However, if nothing has been stored, for example because the frontend is outdated or has never synced its settings, then behavior doesn't change. * The devtools frontend sends no new messages. However, if it did send a new message (e.g. "store this value at this key"), and the backend was outdated, that message would be silently ignored.

…#25569) there are a few bugs where dom representations from SSR aren't identified as Resources when they should be. There are 3 semantics Resource -> hoist to head, deduping, etc... hydratable Component -> SSR'd and hydrated in place non-hydratable Component -> never SSR'd, never hydrated, always inserted on the client this last category is small (non stylesheet) links with onLoad and/or onError async scripts with onLoad and/or onError The reason we have this distinction for now is we need every SSR'd async script to be assumable to be a Resource. we don't currently encode onLoad on the server and so we couldn't otherwise tell if an async script is a Resource or is an async script with an onLoad which would not be a resource. To avoid this ambiguity we never emit the scripts in SSR and assume they need to be inserted on the client. We can explore changes to these semantics in the future or possibly encode some identifier when we want to opt out of resource semantics but still SSR the link or script.

stacked on facebook#25569 On the client noscript already never renders children so no resources will be extracted from this context. On the server we now track if we are in a noscript context and turn off Resource semantics in this scope

…k#25571) In case it's a more modern yet rigid environment.

To avoid confusion. We are patching `fetch`, and only `fetch`, for a small fix scoped to react renders elsewhere, but this code is not it. This code was for the strategy used in the original [React Server Components demo](https://github.com/reactjs/server-components-demo). Which [we announced](https://reactjs.org/blog/2022/06/15/react-labs-what-we-have-been-working-on-june-2022.html) that we're moving away from in favor of [First class support for promises and async/await](reactjs/rfcs#229). We might explore using these package for other instrumentation in the future but not now and not like this.

…book#25543) The old (unstable) mechanism for suspending was to throw a promise. The purpose of throwing is to interrupt the component's execution, and also to signal to React that the interruption was caused by Suspense as opposed to some other error. A flaw is that throwing is meant to be an implementation detail — if code in userspace catches the promise, it can lead to unexpected behavior. With `use`, userspace code does not throw promises directly, but `use` itself still needs to throw something to interrupt the component and unwind the stack. The solution is to throw an internal error. In development, we can detect whether the error was caught by a userspace try/catch block and log a warning — though it's not foolproof, since a clever user could catch the object and rethrow it later. The error message includes advice to move `use` outside of the try/catch block. I did not yet implement the warning in Flight.

In Strict Mode, during development, user functions are double invoked to help detect side effects. Currently, the way we implement this is to completely discard the first pass and start over. Theoretically this should be fine because components are idempotent. However, it's a bit tricky to get right because our implementation (i.e. `renderWithHooks`) is not completely idempotent with respect to internal data structures, like the work-in-progress fiber. In the past we've had to be really careful to avoid subtle bugs — for example, during the initial mount, `setState` functions are bound to the particular hook instances that were created during that render. If we compute new hook instances, we must also compute new children, and they must correspond to each other. This commit addresses a similar issue that came up related to `use`: when something suspends, `use` reuses the promise that was passed during the first attempt. This is itself a form of memoization. We need to be able to memoize the reactive inputs to the `use` call using a hook (i.e. `useMemo`), which means, the reactive inputs to `use` must come from the same component invocation as the output. The solution I've chosen is, rather than double invoke the entire `renderWithHook` function, we should double invoke each individual user function. It's a bit confusing but here's how it works: We will invoke the entire component function twice. However, during the second invocation of the component, the hook state from the first invocation will be reused. That means things like `useMemo` functions won't run again, because the deps will match and the memoized result will be reused. We want memoized functions to run twice, too, so account for this, user functions are double invoked during the *first* invocation of the component function, and are *not* double invoked during the second incovation: - First execution of component function: user functions are double invoked - Second execution of component function (in Strict Mode, during development): user functions are not double invoked. It's hard to explain verbally but much clearer when you run the test cases I've added.

This is a pure refactor, no change to behavior. When a component throws, the work loop can handle that in one of several ways — unwind immediately, wait for microtasks, and so on. I'm about to add another one, too. So I've changed the variable that tracks whether the work loop is suspended from a boolean (workInProgressIsSuspended) to an enum (workInProgressSuspendedReason).

Refactors the logic for handling when the work loop is suspended into separate functions for replaying versus unwinding. This allows us to hoist certain checks into the caller. For example, when rendering due to flushSync, we will always unwind the stack without yielding the microtasks. No intentional behavior change in this commit.

Pure refactor, no change in behavior. Extracts the logic for detecting whether a suspended component will result in a "bad" Suspense fallback into a helper function. An example of a bad Suspense fallback is one that causes already-visible content to disappear. I want to reuse this same logic in the work loop, too.

Found some code that was left over from the experimental useOpaqueIdentifier hook, which we eventually replaced with useId. This deletes it.

) `wasHidden` is evaluted to false if `current` is null. This means Offscreen has never been shown but this code assumes it is going from 'visible' to 'hidden' and unmounts layout effects. To fix this, only unmount layout effects if `current` is not null. I'm not able to repro this problem or write unit test for it. I see this crash bug in production test. The problem with repro is that if Offscreen starts as hidden, it's render is deferred and current is no longer null.

…acebook#25567)

Initial draft. I need to test this more. If a promise is passed to `use`, but the I/O isn't cached, we should still be able to unwrap it. This already worked in Server Components, and during SSR. For Fiber (in the browser), before this fix the state would get lost between attempts unless the promise resolved immediately in a microtask, which requires IO to be cached. This was due to an implementation quirk of Fiber where the state is reset as soon as the stack unwinds. The workaround is to suspend the entire Fiber work loop until the promise resolves. The Server Components and SSR runtimes don't require a workaround: they can maintain multiple parallel child tasks and reuse the state indefinitely across attempts. That's ideally how Fiber should work, too, but it will require larger refactor. The downside of our approach in Fiber is that it won't "warm up" the siblings while you're suspended, but to avoid waterfalls you're supposed to hoist data fetches higher in the tree regardless. But we have other ideas for how we can add this back in the future. (Though again, this doesn't affect Server Components, which already have the ideal behavior.)

…k#25599) `title` is a valid element descendent of `svg`. this PR adds a prohibition on turning titles in svg into Resources. This PR also adds additional warnings if you render something that is almost a Resource inside an svg.

…5609) Makes it slightly more blazing. No host config actually uses null as any useful and we use this as a placeholder value anyway. It's also better for perf since it doesn't let two different hidden classes pass around. It's also a magic place holder that triggers error if we do try to access anything from it.

…r and not leak types (facebook#25610) type validateDOMNesting move `isHostResourceType` to ReactDOMHostConfig type `AncestorInfo` refactor `resourceFormOnly` into `ancestorInfo.containerTagInScope` provide hostContext from reconciler

If a thenable resolves synchronously, `use` should unwrap its result without suspending or interrupting the component's execution.

…om-bindings (facebook#25617)  Following [comment](facebook#25437 (comment)) in facebook#25437 , the external runtime implementation should be moved from `react-dom` to `react-dom-bindings`. I did have a question here: I set the entrypoint to `react-dom/unstable_server-external-runtime.js`, since a.) I was following facebook#25436 as an example and b.) `react-dom-bindings` was missing a `README.md` and `npm/`. This also involved adding the external runtime to `package.json`. However, the external runtime isn't really a `react-dom` entrypoint. Is this change alright, or should I change the bundling code instead? ## How did you test this change?

I don't think we need this anymore. It was added originally because RootSuspended would take priority over RootSuspendedWithDelay. But we've since changed it: any "bad" fallback state is permitted to block a "good" fallback state. The other status flags that this check used to account for are RootDidNotComplete and RootFatalErrored: - RootFatalErrored is like an invariant violation, it means something went really wrong already and we can't recover from it - RootCompleted and RootDidNotComplete are only set at the very end of the work loop, there's no way for renderDidSuspendDelayIfPossible to sneak in after that (at least none that I can think of — it's only called from the render phase) So I think we can just delete this. It's entirely possible there's some scenario I haven't considered, though, which is why I'm submitting this change as its own PR. To preserve the ability to bisect to it later.

…ok#25628) This is a follow up on facebook#25592 There is another condition Offscreen calls `recursivelyTraverseDisappearLayoutEffects` when it shouldn't. Offscreen may be nested. When nested Offscreen is hidden, it should only unmount layout effects if it meets following conditions: 1. This is an update, not first mount. 2. This Offscreen was hidden before. 3. No ancestor Offscreen is hidden. Previously, we were not accounting for the third condition.

## Summary This is to support two new reconciler work tags `HostSingleton` and `HostResource` introduced in PRs facebook#25243 facebook#25426. The behavior is described below. I also renamed an option in components settings from an internal concept "host" to more understood "dom nodes" ## How did you test this change? Tested on the latest Vercel playground app https://github.com/vercel/app-playground/ Before the change, devtools cannot show correct display name for these new elements. Also, some unnecessary internal details are exposed to users. <img width="1395" alt="image" src="https://user-images.githubusercontent.com/1001890/199578181-c4e4ea74-baa1-4507-83d0-91a62ad7de5f.png"> After the change, the display names are correctly shown and the "state" would always be hidden in the detail view. <img width="1417" alt="image" src="https://user-images.githubusercontent.com/1001890/199578442-adc1951d-7d5b-4b84-ad64-85bcf7a8ebcc.png"> These elements will also be hidden just like other native dom elements (e.g. `<div>`) <img width="836" alt="image" src="https://user-images.githubusercontent.com/1001890/199578598-2dfacf64-ddc9-42b5-a246-dd0b09f629af.png">

…ender phase (facebook#25578) Fix facebook#25565

Nested Offscreens can run into a case where outer Offscreen is revealed while inner one is hidden in a single commit. This is an edge case that was previously missed. We need to prevent call to disappear layout effects. When we go from state: ```jsx <Offscreen mode={'hidden'}> // outer offscreen <Offscreen mode={'visible'}> // inner offscreen {children} </Offscreen> </Offscreen> ``` To following. Notice that visibility of each offscreen flips. ```jsx <Offscreen mode={'visible'}> // outer offscreen <Offscreen mode={'hidden'}> // inner offscreen {children} </Offscreen> </Offscreen> ``` Inner offscreen must not call `recursivelyTraverseDisappearLayoutEffects`. Check unit tests for an example of this.

## Summary This PR is to fix a bug: an "element cannot be found" error when hydrating Server Components ### The problem <img width="1061" alt="image" src="https://user-images.githubusercontent.com/1001890/201206046-ac32a5e3-b08a-4dc2-99f4-221dad504b28.png"> To reproduce: 1. setting up a vercel next.js 13 playground locally https://github.com/vercel/app-playground 2. visit http://localhost:3000/loading 3. click "electronics" button to navigate to http://localhost:3000/loading/electronics to trigger hydrating 4. inspect one of the skeleton card UI from React DevTools extension ### The root cause & fix This bug was introduced in facebook#22527. When syncing reconciler changes, the value of `Hydrating` was copied from another variable `Visibility` (one more zero in the binary number). To avoid this kind of issue in the future, a new file `ReactFiberFlags` is created following the same format of the one in reconciler, so that it's easier to sync the number without making mistakes. The reconciler fiber flag file is also updated to reflect which of the flags are used in devtools ## How did you test this change? I build it locally and the bug no longer exist on http://localhost:3000/loading

@tyao1

…oundary (facebook#25692) This just removes the error but the underlying issue is still there, and it's likely that the best course of action is to not update in effects and to wrap most updates in startTransition. However, that's more of a performance concern which is not something we generally do even in recoverable errors since they're less actionable and likely belong in another channel. It is also likely that in many cases this happens so rarely because you have to interact quickly enough that it can often be ignored. After changes to other parts of the model, this only happens for sync/discrete updates. There are three scenarios that can happen: - We replace a server rendered fallback with a client rendered fallback. Other than this potentially causing some flickering in the loading state, it's not a big deal. - We replace the server rendered content with a client side fallback if this suspends on the client. This is in line with what would happen anyway. We will loose state of forms which is not intended semantics. State and animations etc would've been lost anyway if it was client-side so that's not a concern. - We replace the server rendered content with a client side rendered tree and lose selection/state and form state. While the content looks the same, which is unfortunate. In most scenarios it's a bad loading state but it's the same scenario as flushing sync client-side. So it's not so bad. The big change here is that we consider this a bug of React that we should fix. Therefore it's not actionable to users today because it should just get fixed. So we're removing the error early. Although anyone that has fixed these issues already are probably better off for it. To fix this while still hydrating we need to be able to rewind a sync tree and then replay it. @tyao1 is going to add a Sync hydration lane. This is will allow us to rewind the tree when we hit this state, and replay it given the previous Context, hydrate and then reapply the update. The reason we didn't do this originally is because it causes sync mode to unwind where as for backwards compatibility we didn't want to cause that breaking semantic - outside Suspense boundaries - and we don't want that semantic longer term. We're only do this as a short term fix. We should also have a way to leave a partial tree in place. If the sync hydration lane suspends, we should be able to switch to a client side fallback without throwing away the state of the DOM and then hydrate later. We now know how we want to fix this longer term. We're going to move all Contexts into resumable trees like what Fizz/Flight does. That way we can leave the original Context at the hydration boundaries and then resume from there. That way the rewinding would never happen even in the existence of a sync hydration lane which would only apply locally to the dehydrated tree. So the steps are 1) remove the error 2) add the sync hydration lane with rewinding 3) Allow hiding server-rendered content while still not hydrated 4) add resumable contexts at these boundaries. Fixes facebook#25625 and facebook#24959.

Some old environments like IE11 or very old versions of jsdom are missing `getRootNode()`. Use feature detection to fall back to `ownerDocuments` in these environments that also won't be supporting shadow DOM anyway.

[//]: # (dependabot-start) ⚠️ **Dependabot is rebasing this PR** ⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Bumps [loader-utils](https://github.com/webpack/loader-utils) from 1.4.0 to 1.4.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/webpack/loader-utils/releases">loader-utils's releases</a>.</em></p> <blockquote> <h2>v1.4.2</h2> <h3><a href="https://github.com/webpack/loader-utils/compare/v1.4.1...v1.4.2">1.4.2</a> (2022-11-11)</h3> <h3>Bug Fixes</h3> <ul> <li>ReDoS problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/226">#226</a>) (<a href="https://github.com/webpack/loader-utils/commit/17cbf8fa8989c1cb45bdd2997aa524729475f1fa">17cbf8f</a>)</li> </ul> <h2>v1.4.1</h2> <h3><a href="https://github.com/webpack/loader-utils/compare/v1.4.0...v1.4.1">1.4.1</a> (2022-11-07)</h3> <h3>Bug Fixes</h3> <ul> <li>security problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/220">#220</a>) (<a href="https://github.com/webpack/loader-utils/commit/4504e34c4796a5836ef70458327351675aed48a5">4504e34</a>)</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/webpack/loader-utils/blob/v1.4.2/CHANGELOG.md">loader-utils's changelog</a>.</em></p> <blockquote> <h3><a href="https://github.com/webpack/loader-utils/compare/v1.4.1...v1.4.2">1.4.2</a> (2022-11-11)</h3> <h3>Bug Fixes</h3> <ul> <li>ReDoS problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/226">#226</a>) (<a href="https://github.com/webpack/loader-utils/commit/17cbf8fa8989c1cb45bdd2997aa524729475f1fa">17cbf8f</a>)</li> </ul> <h3><a href="https://github.com/webpack/loader-utils/compare/v1.4.0...v1.4.1">1.4.1</a> (2022-11-07)</h3> <h3>Bug Fixes</h3> <ul> <li>security problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/220">#220</a>) (<a href="https://github.com/webpack/loader-utils/commit/4504e34c4796a5836ef70458327351675aed48a5">4504e34</a>)</li> </ul> <p></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/webpack/loader-utils/commit/331ad5067d9a1a7b8d646692e6959639969210d1"><code>331ad50</code></a> chore(release): 1.4.2</li> <li><a href="https://github.com/webpack/loader-utils/commit/17cbf8fa8989c1cb45bdd2997aa524729475f1fa"><code>17cbf8f</code></a> fix: ReDoS problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/226">#226</a>)</li> <li><a href="https://github.com/webpack/loader-utils/commit/8f082b39f6903929f30fe29dab34f4d9c7ef070a"><code>8f082b3</code></a> chore(release): 1.4.1</li> <li><a href="https://github.com/webpack/loader-utils/commit/4504e34c4796a5836ef70458327351675aed48a5"><code>4504e34</code></a> fix: security problem (<a href="https://github-redirect.dependabot.com/webpack/loader-utils/issues/220">#220</a>)</li> <li>See full diff in <a href="https://github.com/webpack/loader-utils/compare/v1.4.0...v1.4.2">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=loader-utils&package-manager=npm_and_yarn&previous-version=1.4.0&new-version=1.4.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/facebook/react/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

This micro-optimization never made sense and less so now that they're rare. This still initializes the class with a shared immutable object in the constructor - which is also what createClass() does. Then we override it during mount. This is done in case someone messes up the initialization of the super() constructor for example, which was more common in polyfills. This change means that if a ref is initialized during the constructor itself it wouldn't be lazily initialized but that's not user code that does it, it's React so that shouldn't happen. This makes string refs codemoddable as described in. facebook#25334

This improves the error message a bit and ensures that we recommend putting the key first, not last, which ensures that the faster `jsx-runtime` is used. This only affects the modern "automatic" JSX transform.

…e) (facebook#25383) ## Summary Alternate to facebook#25334 without any prod runtime changes i.e. the proposed codemod in https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#deprecate-string-refs-and-remove-production-mode-_owner-field would not work. ## How did you test this change? - [x] CI - [x] `yarn test` with and without `warnAboutStringRefs`

…acebook#25699) This also fixes a gap where were weren't warning on memo components.

When an update flows into a dehydrated boundary, React cannot apply the update until the boundary has finished hydrating. The way this currently works is by scheduling a slightly higher priority task on the boundary, using a special lane that's reserved only for this purpose. Because the task is slightly higher priority, on the next turn of the work loop, the Scheduler will force the work loop to yield (i.e. shouldYield starts returning `true` because there's a higher priority task). The downside of this approach is that it only works when time slicing is enabled. It doesn't work for synchronous updates, because the synchronous work loop does not consult the Scheduler on each iteration. We plan to add support for selective hydration during synchronous updates, too, so we need to model this some other way. I've added a special internal exception that can be thrown to force the work loop to interrupt the work-in-progress tree. Because it's thrown from a React-only execution stack, throwing isn't strictly necessary — we could instead modify some internal work loop state. But using an exception means we don't need to check for this case on every iteration of the work loop. So doing it this way moves the check out of the fast path. The ideal implementation wouldn't need to unwind the stack at all — we should be able to hydrate the subtree and then apply the update all within a single render phase. This is how we intend to implement it in the future, but this requires a refactor to how we handle "stack" variables, which are currently pushed to a per-render array. We need to make this stack resumable, like how context works in Flight and Fizz.

Before suspending, check if there are other pending updates that might possibly unblock the suspended component. If so, interrupt the current render and switch to working on that. This logic was already implemented for the old "throw a Promise" Suspense but has to be replicated for `use` because it suspends the work loop much earlier. I'm getting a little anxious about the divergence between the two Suspense patterns. I'm going to look into enabling the new behavior for the old pattern so that we can unify the implementations.

When replaying a suspended function components, we want to reuse the hooks that were computed during the original render. Currently we reset the state of the hooks right after the component suspends (or throws an error). This is too early because it doesn't give us an opportunity to wait for the promise to resolve. This refactors the work loop to reset the hooks right before unwinding instead of right after throwing. It doesn't include any other changes yet, so there should be no observable behavioral change.

Tiny refactor to refine the work loop variable so Flow knows it's not null when we access it in replaySuspendedUnitOfWork.

Currently, if you call setState in render, you must render the exact same hooks as during the first render pass. I'm about to add a behavior where if something suspends, we can reuse the hooks from the previous attempt. That means during initial render, if something suspends, we should be able to reuse the hooks that were already created and continue adding more after that. This will error in the current implementation because of the expectation that every render produces the same list of hooks. In this commit, I've changed the logic to allow more hooks to be added when replaying. But only during a mount — if there's already a current fiber, then the logic is unchanged, because we shouldn't add any additional hooks that aren't in the current fiber's list. Mounts are special because there's no current fiber to compare to. I haven't change any other behavior yet. The reason I've put this into its own step is there are a couple tests that intentionally break the Hook rule, to assert that React errors in these cases, and those happen to be coupled to the behavior. This is undefined behavior that is always accompanied by a warning and/or error. So the change should be safe.

When a component suspends, under some conditions, we can wait for the data to resolve and replay the component without unwinding the stack or showing a fallback in the interim. When we do this, we reuse the promises that were unwrapped during the previous attempts, so that if they aren't memoized, the result can still be used. We should do the same for all hooks. That way, if you _do_ memoize an async function call with useMemo, it won't be called again during the replay. This effectively gives you a local version of the functionality provided by `cache`, using the normal memoization patterns that have long existed in React.

Now that hook state is preserved while the work loop is suspended, we don't need to track the thenable state in the work loop. We can track it alongside the rest of the hook state. Before deleting the thenable state variable from the work loop, I need to remove the other places where it's referenced. One of them is `isThenableStateResolved`. This grabs the last thenable from the array and checks if it has resolved. This was a pointless indirection anyway. The thenable is already stored as `workInProgressThrownValue`. So we can check that directly.

Now that hook state is preserved while the work loop is suspended, we don't need to track the thenable state in the work loop. We can track it alongside the rest of the hook state. This is a nice simplification and also aligns better with how it works in Fizz and Flight. The promises will still be cleared when the component finishes rendering (either complete or unwind). In the future, we could stash the promises on the fiber and reuse them during an update. However, this would only work for `use` calls that occur before an prop/state/context is processed, because `use` calls can only be assumed to execute in the same order if no other props/state/context have changed. So it might not be worth doing until we have finer grained memoization.

## Summary  For more context: facebook#25692 Based on facebook#25695. This PR adds the `SyncHydrationLane` so we rewind on sync updates during selective hydration. Also added tests for ContinuouseHydration and DefaultHydration lanes. ## How did you test this change?  yarn test

rbalicki2 and others added 30 commits October 25, 2022 13:25

Try assigning fetch to globalThis if global assignment fails (faceboo…

28a574e

…k#25571) In case it's a more modern yet rigid environment.

Clean up vestige of useOpaqueIdentifier (facebook#25587)

2ac77aa

Found some code that was left over from the experimental useOpaqueIdentifier hook, which we eventually replaced with useId. This deletes it.

Fix type check for null (facebook#25595)

765805b

[ServerRenderer] Setup for adding data attributes streaming format (f…

6883d79

…acebook#25567)

Unwrap sync resolved thenables without suspending (facebook#25615)

1a90262

If a thenable resolves synchronously, `use` should unwrap its result without suspending or interrupting the component's execution.

Fix useSyncExternalStore dropped update when state is dispatched in r…

1e3e30d

…ender phase (facebook#25578) Fix facebook#25565

[react-float] feature detect getRootNode (facebook#25689)

c343f80

Some old environments like IE11 or very old versions of jsdom are missing `getRootNode()`. Use feature detection to fall back to `ownerDocuments` in these environments that also won't be supporting shadow DOM anyway.

sebmarkbage and others added 14 commits November 16, 2022 15:16

Turn on key spread warning in jsx-runtime for everyone (facebook#25697)

07f46ec

This improves the error message a bit and ensures that we recommend putting the key first, not last, which ensures that the faster `jsx-runtime` is used. This only affects the modern "automatic" JSX transform.

Enable warning for defaultProps on function components for everyone (f…

7b17f7b

…acebook#25699) This also fixes a gap where were weren't warning on memo components.

Pass ThenableState to replaySuspendedUnitOfWork

5eb78d0

Tiny refactor to refine the work loop variable so Flow knows it's not null when we access it in replaySuspendedUnitOfWork.

Test for double invoke effect of useLayoutEffect

b0a6f66

kassens closed this Nov 19, 2022

kassens deleted the double-invoke-test branch May 8, 2023 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

double invoke test #1

double invoke test #1

kassens commented Nov 17, 2022

double invoke test #1

double invoke test #1

Conversation

kassens commented Nov 17, 2022