We're reverting the stack of changes that this code belongs to in order
to unblock the sync to Meta's internal codebase. We will attempt to
re-land once the sync is unblocked.
I have not yet verified that this fixes the error that were reported
internally. I will do that before landing.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn debug-test --watch TestName`, open
`chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
This is discovered by @acdlite.
In dev we replay errors so debugger will treat them as uncaught errors,
but we need to ignore internal exceptions.
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## How did you test this change?
manually console.log in some tests, and noticed replay didn't happen.
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
We've heard from multiple contributors that the Reconciler forking
mechanism was confusing and/or annoying to deal with. Since it's
currently unused and there's no immediate plans to start using it again,
this removes the forking.
Fully removing the fork is split into 2 steps to preserve file history:
**#25774 previous PR that did the bulk of the work:**
- remove `enableNewReconciler` feature flag.
- remove `unstable_isNewReconciler` export
- remove eslint rules for cross fork imports
- remove `*.new.js` files and update imports
- merge non-suffixed files into `*.old` files where both exist
(sometimes types were defined there)
**This PR**
- rename `*.old` files
* Add another test for #18515 using pings
Adds a regression test for the same underlying bug as #18515 but using
pings.
Test already passes, but I confirmed it fails if you revert the fix
in #18515.
* Set nextPendingLevel after commit, too
* Reproduce a bug where `flushDiscreteUpdates` causes fallback never to be committed
* Ping suspended level when canceling its timer
Make sure the suspended level is marked as pinged so that we return back
to it later, in case the render we're about to start gets aborted.
Generally we only reach this path via a ping, but we shouldn't assume
that will always be the case.
* Clear finished discrete updates during commit phase
If a root is finished at a priority lower than that of the latest pending discrete
updates on it, these updates must have been finished so we can clear them now.
Otherwise, a later call of `flushDiscreteUpdates` would start a new empty render
pass which may cause a scheduled timeout to be cancelled.
* Add TODO
Happened to find this while writing a test. A JSX element comparison
failed because one of them elements had a functional component as an
owner, which should ever happen.
I'll add a regression test later.
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Add useOpaqueIdentifier Hook
We currently use unique IDs in a lot of places. Examples are:
* `<label for="ID">`
* `aria-labelledby`
This can cause some issues:
1. If we server side render and then hydrate, this could cause an
hydration ID mismatch
2. If we server side render one part of the page and client side
render another part of the page, the ID for one part could be
different than the ID for another part even though they are
supposed to be the same
3. If we conditionally render something with an ID , this might also
cause an ID mismatch because the ID will be different on other
parts of the page
This PR creates a new hook `useUniqueId` that generates a different
unique ID based on whether the hook was called on the server or client.
If the hook is called during hydration, it generates an opaque object
that will rerender the hook so that the IDs match.
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Add feature flag
* Split stack from current fiber
You can get stack from any fiber, not just current.
* Refactor description of component frames
These should use fiber tags for switching. This also puts the relevant code
behind DEV flags.
* We no longer expose StrictMode in component stacks
They're not super useful and will go away later anyway.
* Update tests
Context is no longer part of SSR stacks. This was already the case on the
client.
forwardRef no longer is wrapped on the stack. It's still in getComponentName
but it's probably just noise in stacks. Eventually we'll remove the wrapper
so it'll go away anyway. If we use native stack frames they won't have this
extra wrapper.
It also doesn't pick up displayName from the outer wrapper. We could maybe
transfer it but this will also be fixed by removing the wrapper.
* Forward displayName onto the inner function for forwardRef and memo in DEV
This allows them to show up in stack traces.
I'm not doing this for lazy because lazy is supposed to be called on the
consuming side so you shouldn't assign it a name on that end. Especially
not one that mutates the inner.
* Use multiple instances of the fake component
We mutate the inner component for its name so we need multiple copies.
* Enable prefer-const rule
Stylistically I don't like this but Closure Compiler takes advantage of
this information.
* Auto-fix lints
* Manually fix the remaining callsites
React can't directly detect a memory leak, but there are some clues that warn about one. One of these clues is when an unmounted React component tries to update its state. For example, if a component forgets to remove an event listener when unmounting, that listener may be called later and try to update state, at which point React would warn about the potential leak.
Warning signals like this are more useful if they're strong. For this reason, it's good to always avoid updating state from inside of an effect's cleanup function. Even when you know there is no potential leak, React has no way to know and so it will warn anyway.
In most cases we suggest moving state updates to the useEffect() body instead (to avoid triggering the warning). This works so long as the component is updating its own state (or the state of a descendant). However this will not work when a component updates its parent state in a cleanup function. If such a component is unmounted but its parent remains mounted, the state will be incorrect. For this reason, we now avoid showing the warning if a component is updating an ancestor.
* Formalize the Wakeable and Thenable types
We use two subsets of Promises throughout React APIs. This introduces
the smallest subset - Wakeable. It's the thing that you can throw to
suspend. It's something that can ping.
I also use a shared type for Thenable in the cases where we expect a value
so we can be a bit more rigid with our us of them.
* Make Chunks into Wakeables instead of using native Promises
This value is just going from here to React so we can keep it a lighter
abstraction throughout.
* Renamed thenable to wakeable in variable names
* Rename lower case isomorphic default exports modules to upper case named exports
We're somewhat inconsistent here between e.g. ReactLazy and memo.
Let's pick one.
This also moves the responder, fundamental, scope creators from shared
since they're isomorphic and same as the other creators.
* Move some files that are specific to the react-reconciler from shared
Individual renderers are allowed to deep require into the reconciler.
* Move files specific to react-dom from shared
react-interactions is right now dom specific (it wasn't before) so we can
type check it together with other dom stuff. Avoids the need for
a shared ReactDOMTypes to be checked by RN for example.
* Move ReactWorkTags to the reconciler
* Move createPortal to export from reconciler
Otherwise Noop can't access it since it's not allowed deep requires.
* Change the warning to not say "function body"
This warning is more generic and may happen with class components too.
* Dedupe by the rendering component
* Don't warn outside of render
* improve error message for cross-functional component updates
* correctly use %s by quoting it
* use workInProgress and lint
* add test assertion
* fix test
* Improve the error message
Co-authored-by: Dan Abramov <dan.abramov@me.com>
Don't warn about unmounted state updates from within passive destroy function
* Fixed test conditional. (It broke after recent variant refactor.)
* Changed warning wording for setState from within useEffect destroy callback
useMutableSource hook
useMutableSource() enables React components to safely and efficiently read from a mutable external source in Concurrent Mode. The API will detect mutations that occur during a render to avoid tearing and it will automatically schedule updates when the source is mutated.
RFC: reactjs/rfcs#147
* Implemented Profiler onCommit() and onPostCommit() hooks
* Added enableProfilerCommitHooks feature flag for commit hooks
* Moved onCommit and onPassiveCommit behind separate feature flag
* Bugfix: Expiring a partially completed tree (#17926)
* Failing test: Expiring a partially completed tree
We should not throw out a partially completed tree if it expires in the
middle of rendering. We should finish the rest of the tree without
yielding, then finish any remaining expired levels in a single batch.
* Check if there's a partial tree before restarting
If a partial render expires, we should stay in the concurrent path
(performConcurrentWorkOnRoot); we'll stop yielding, but the rest of the
behavior remains the same.
We will only revert to the sync path (performSyncWorkOnRoot) when
starting on a new level.
This approach prevents partially completed concurrent work from
being discarded.
* New test: retry after error during expired render
* Regression: Expired partial tree infinite loops
Adds regression tests that reproduce a scenario where a partially
completed tree expired then fell into an infinite loop.
The code change that exposed this bug made the assumption that if you
call Scheduler's `shouldYield` from inside an expired task, Scheduler
will always return `false`. But in fact, Scheduler sometimes returns
`true` in that scenario, which is a bug.
The reason it worked before is that once a task timed out, React would
always switch to a synchronous work loop without checking `shouldYield`.
My rationale for relying on `shouldYield` was to unify the code paths
between a partially concurrent render (i.e. expires midway through) and
a fully concurrent render, as opposed to a render that was synchronous
the whole time. However, this bug indicates that we need a stronger
guarantee within React for when tasks expire, given that the failure
case is so catastrophic. Instead of relying on the result of a dynamic
method call, we should use control flow to guarantee that the work is
synchronously executed.
(We should also fix the Scheduler bug so that `shouldYield` always
returns false inside an expired task, but I'll address that separately.)
* Always switch to sync work loop when task expires
Refactors the `didTimeout` check so that it always switches to the
synchronous work loop, like it did before the regression.
This breaks the error handling behavior that I added in 5f7361f (an
error during a partially concurrent render should retry once,
synchronously). I'll fix this next. I need to change that behavior,
anyway, to support retries that occur as a result of `flushSync`.
* Retry once after error even for sync renders
Except in legacy mode.
This is to support the `useOpaqueReference` hook, which uses an error
to trigger a retry at lower priority.
* Move partial tree check to performSyncWorkOnRoot
* Factor out render phase
Splits the work loop and its surrounding enter/exit code into their own
functions. Now we can do perform multiple render phase passes within a
single call to performConcurrentWorkOnRoot or performSyncWorkOnRoot.
This lets us get rid of the `didError` field.
I recently landed a change to the timing of passive effect cleanup functions during unmount (see #17925). This change defers flushing of passive effects for unmounted components until later (whenever we next flush pending passive effects).
Since this change increases the likelihood of a (not actionable) state update warning for unmounted components, I've suppressed that warning for Fibers that have scheduled passive effect unmounts pending.
This warning already exists for class components, but not for functions.
It does not apply to render phase updates to the same component, which
have special semantics that we do support.
* Re-throw errors thrown by the renderer at the root
React treats errors thrown at the root as a fatal because there's no
parent component that can capture it. (This is distinct from an
"uncaught error" that isn't wrapped in an error boundary, because in
that case we can fall back to deleting the whole tree -- not great, but
at least the error is contained to a single root, and React is left in a
consistent state.)
It turns out we didn't have a test case for this path. The only way it
can happen is if the renderer's host config throws. We had similar test
cases for host components, but none for the host root.
This adds a new test case and fixes a bug where React would keep
retrying the root because the `workInProgress` pointer was not advanced
to the next fiber. (Which in this case is `null`, since it's the root.)
We could consider in the future trying to gracefully exit from certain
types of root errors without leaving React in an inconsistent state. For
example, we should be able to gracefully exit from errors thrown in the
begin phase. For now, I'm treating it like an internal invariant and
immediately exiting.
* Add comment
* Split recent passive effects changes into 2 flags
Separate flags can now be used to opt passive effects into:
1) Deferring destroy functions on unmount to subsequent passive effects flush
2) Running all destroy functions (for all fibers) before create functions
This allows us to test the less risky feature (2) separately from the more risky one.
* deferPassiveEffectCleanupDuringUnmount is ignored unless runAllPassiveEffectDestroysBeforeCreates is true
* Flush all passive destroy fns before calling create fns
Previously we only flushed destroy functions for a single fiber.
The reason this is important is that interleaving destroy/create effects between sibling components might cause components to interfere with each other (e.g. a destroy function in one component may unintentionally override a ref value set by a create function in another component).
This PR builds on top of the recently added deferPassiveEffectCleanupDuringUnmount kill switch to separate passive effects flushing into two separate phases (similar to layout effects).
* Change passive effect flushing to use arrays instead of lists
This change offers a small advantage over the way we did things previous: it continues invoking destroy functions even after a previous one errored.
* Flush useEffect clean up functions in the passive effects phase
This is a change in behavior that may cause broken product code, so it has been added behind a killswitch (deferPassiveEffectCleanupDuringUnmount)
* Avoid scheduling unnecessary callbacks for cleanup effects
Updated enqueuePendingPassiveEffectDestroyFn() to check rootDoesHavePassiveEffects before scheduling a new callback. This way we'll only schedule (at most) one.
* Updated newly added test for added clarity.
* Cleaned up hooks effect tags
We previously used separate Mount* and Unmount* tags to track hooks work for each phase (snapshot, mutation, layout, and passive). This was somewhat complicated to trace through and there were man tag types we never even used (e.g. UnmountLayout, MountMutation, UnmountSnapshot). In addition to this, it left passive and layout hooks looking the same after renders without changed dependencies, which meant we were unable to reliably defer passive effect destroy functions until after the commit phase.
This commit reduces the effect tag types to only include Layout and Passive and differentiates between work and no-work with an HasEffect flag.
* Disabled deferred passive effects flushing in OSS builds for now
* Split up unmount and mount effects list traversal
This reverts commit 01974a867c.
* Failing test: Expiring a partially completed tree
We should not throw out a partially completed tree if it expires in the
middle of rendering. We should finish the rest of the tree without
yielding, then finish any remaining expired levels in a single batch.
* Check if there's a partial tree before restarting
If a partial render expires, we should stay in the concurrent path
(performConcurrentWorkOnRoot); we'll stop yielding, but the rest of the
behavior remains the same.
We will only revert to the sync path (performSyncWorkOnRoot) when
starting on a new level.
This approach prevents partially completed concurrent work from
being discarded.
* New test: retry after error during expired render
* Failing test: Expiring a partially completed tree
We should not throw out a partially completed tree if it expires in the
middle of rendering. We should finish the rest of the tree without
yielding, then finish any remaining expired levels in a single batch.
* Check if there's a partial tree before restarting
If a partial render expires, we should stay in the concurrent path
(performConcurrentWorkOnRoot); we'll stop yielding, but the rest of the
behavior remains the same.
We will only revert to the sync path (performSyncWorkOnRoot) when
starting on a new level.
This approach prevents partially completed concurrent work from
being discarded.
* New test: retry after error during expired render
* Remove renderPhaseUpdates Map
Follow up to #17484, which was reverted due to a bug found in www.
* Failing test: Dropped updates
When resetting render phase updates after a throw, we should only clear
the pending queue of hooks that were already processed.
* Fix non-render-phase updates being dropped
Detects if a queue has been processed by whether the hook was cloned.
If we change the implementation to an array instead of a list, we'll
need some other mechanism to determine whether the hook was processed.
* Regression test: startTransition in render phase
useTransition uses the state hook as part of its implementation, so we
need to fork it in the dispatcher used for re-renders, too.
* Update Flow to 0.84
* Fix violations
* Use inexact object syntax in files from fbsource
* Fix warning extraction to use a modern parser
* Codemod inexact objects to new syntax
* Tighten types that can be exact
* Revert unintentional formatting changes from codemod
* Add feature flags
* Add Chunk type and constructor
* Wire up Chunk support in the reconciler
* Update reconciler to reconcile Chunks against the render method
This allows the query and args to be updated.
* Drop the ref. Chunks cannot have refs anyway.
* Add Chunk checks in more missing cases
* Rename secondArg
* Add test and fix lazy chunks
Not really a supported use case but for consistency I guess.
* Fix fragment test
* Replace all warning/lowPriWarning with console calls
* Replace console.warn/error with a custom wrapper at build time
* Fail the build for console.error/warn() where we can't read the stack
* Render phase updates can now be extracted from the pending queue
* Use a custom dispatcher for the second render pass
* Discard render phase updates if component throws
When aborting a render, we also need to throw out render phase updates.
Remove the updates from the queues so they do not persist to the next
render. We already did a single pass through the whole list of hooks, so
we know that any pending updates must have been dispatched during the
render phase. The ones that were dispatched before we started rendering
were already transferred to the current hook's queue.
* prep for codemod
* prep warnings
* rename lint rules
* codemod for ifs
* shim www functions
* Handle more cases in the transform
* Thanks De Morgan
* Run the codemod
* Delete the transform
* Fix up confusing conditions manually
* Fix up www shims to match expected API
* Also check for low-pri warning in the lint rule
* Refactor Update Queues to Fix Rebasing Bug
Fixes a bug related to rebasing updates. Once an update has committed,
it should never un-commit, even if interrupted by a higher priority
update. The fix includes a refactor of how update queues work.
This commit is a combination of two PRs:
- #17483 by @sebmarkbage refactors the hook update queue
- #17510 by @acdlite refactors the class and root update queue
Landing one without the other would cause state updates to sometimes be
inconsistent across components, so I've combined them into a single
commit in case they need to be reverted.
Co-authored-by: Sebastian Markbåge <sema@fb.com>
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Initialize update queue object on mount
Instead of lazily initializing update queue objects on the first update,
class and host root queues are created on mount. This simplifies the
logic for appending new updates and matches what we do for hooks.
Reverts b617db3d96.
Found some bugs when attempting to land in www. Reverting to fix master.
I'll land again *after* the change successfully land downstream.
Fixes a bug related to rebasing updates. Once an update has committed,
it should never un-commit, even if interrupted by a higher priority
update. The fix includes a refactor of how update queues work.
This commit is a combination of two PRs:
- #17483 by @sebmarkbage refactors the hook update queue
- #17510 by @acdlite refactors the class and root update queue
Landing one without the other would cause state updates to sometimes be
inconsistent across components, so I've combined them into a single
commit in case they need to be reverted.
Co-authored-by: Sebastian Markbåge <sema@fb.com>
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Replace Babel plugin with an ESLint plugin
* Fix ESLint rule violations
* Move shared conditions higher
* Test formatting nits
* Tweak ESLint rule
* Bugfix: inside else branch, 'if' tests are not satisfactory
* Use a stricter check for exactly if (__DEV__)
This makes it easier to see what's going on and matches dominant style in the codebase.
* Fix remaining files after stricter check
Andrew Clark
Tianyu Yao
Jan Kassens
jddxfandAndrew Clark
Luna RuanandAndrew Clark
Sebastian Markbåge
Brian Vaughn
Dan Abramov
Jovi De CroockandDan Abramov
Christoph Nakazawa
Dominic Gannaway
Laura buns
Laura buns