* BUG: ReactPartialRenderer / New Context polutes mutable global state
The new context API stores the provided values on the shared context instance. When used in a synchronous context, this is not an issue. However when used in an concurrent context this can cause a "push provider" from one react render to have an effect on an unrelated concurrent react render.
I've encountered this bug in production when using renderToNodeStream, which asks ReactPartialRenderer for bytes up to a high water mark before yielding. If two Node Streams are created and read from in parallel, the state of one can polute the other.
I wrote a failing test to illustrate the conditions under which this happens.
I'm also concerned that the experimental concurrent/async React rendering on the client could suffer from the same issue.
* Use unique thread ID for each partial render to access Context
This first adds an allocator that keeps track of a unique ThreadID index
for each currently executing partial renderer. IDs are not just growing
but are reused as streams are destroyed.
This ensures that IDs are kept nice and compact.
This lets us use an "array" for each Context object to store the current
values. The look up for these are fast because they're just looking up
an offset in a tightly packed "array".
I don't use an actual Array object to store the values. Instead, I rely
on that VMs (notably V8) treat storage of numeric index property access
as a separate "elements" allocation.
This lets us avoid an extra indirection.
However, we must ensure that these arrays are not holey to preserve this
feature.
To do that I store the _threadCount on each context (effectively it takes
the place of the .length property on an array).
This lets us first validate that the context has enough slots before we
access the slot. If not, we fill in the slots with the default value.
Oopsie!
This could have been avoided if our types were modeled correctly with
Flow (using a disjoint union).
Fuzz tester didn't catch it because it does not generate cases where
a Suspense component mounts with no children. I'll update it.
* Don't warn if an unmounted component is pinged
* Suspense fuzz tester
The fuzzer works by generating a random tree of React elements. The tree
two types of custom components:
- A Text component suspends rendering on initial mount for a fuzzy
duration of time. It may update a fuzzy number of times; each update
supsends for a fuzzy duration of time.
- A Container component wraps some children. It may remount its children
a fuzzy number of times, by updating its key.
The tree may also include nested Suspense components.
After this tree is generated, the tester sets a flag to temporarily
disable Text components from suspending. The tree is rendered
synchronously. The output of this render is the expected output.
Then the tester flips the flag back to enable suspending. It renders the
tree again. This time the Text components will suspend for the amount of
time configured by the props. The tester waits until everything has
resolved. The resolved output is then compared to the expected output
generated in the previous step.
Finally, we render once more, but this time in concurrent mode. Once
again, the resolved output is compared to the expected output.
I tested by commenting out various parts of the Suspense implementation
to see if broke in the expected way. I also confirmed that it would have
caught #14133, a recent bug related to deletions.
* When a generated test case fails, log its input
* Moar fuzziness
Adds more fuzziness to the generated tests. Specifcally, introduces
nested Suspense cases, where the fallback of a Suspense component
also suspends.
This flushed out a bug (yay!) whose test case I've hard coded.
* Use seeded random number generator
So if there's a failure, we can bisect.
* Add failing test for ping on unmounted component
We had a test for this, but not outside of concurrent mode :)
* Don't warn if an unmounted component is pinged
* Fixed `treeBaseDuration` by propagating its value from the suspended tree to the Fragment React temporarily wraps around it when showing the fallback UI.
* Fixed `actualDuration` by recording elapsed profiler time in the event of an error.
* Fixed `actualDuration` in concurrent mode by propagating the time spent rendering the suspending component to its parent.
Also updated ReactSuspensePlaceholder-test.internal to cover these new cases.
Fixes a bug where deletion effects in the primary tree were dropped
before entering the second render pass.
Because we no longer reset the effect list after the first render pass,
I've also moved the deletion of the fallback children to the complete
phase, after the tree successfully renders without suspending.
Will need to revisit this heuristic when we implement resuming.
* Recover from errors with a boundary in completion phase
* Use a separate field for completing unit of work
* Use a simpler fix with one boolean
* Reoder conditions
* Clarify which paths are DEV-only
* Move duplicated line out
* Make it clearer this code is DEV-only
Adds a check to the existing fuzz tester to confirm that the props are
set to the latest values in the commit phase. Only checks
componentDidUpdate; we already have unit tests for the other lifecycles,
so I think this is good enough. This is only a redundancy.
* Resolve defaultProps for Lazy components
* Make test fail again
* Undo the partial fix
* Make test output more compact
* Add a separate failing test for sync mode
* Clean up tests
* Add another update to both tests
* Resolve props for commit phase lifecycles
* Resolve prevProps for begin phase lifecycles
* Resolve prevProps for pre-commit lifecycles
* Only resolve props if element type differs
* Fix Flow
* Don't set instance.props/state during commit phase
This is an optimization. I'm not sure it's entirely safe. It's probably worth running internal tests and see if we can ever trigger a case where they're different.
This can mess with resuming.
* Keep setting instance.props/state before unmounting
This reverts part of the previous commit. It broke a test that verifies we use current props in componentWillUnmount if the fiber unmounts due to an error.
Setting to null isn't correct; setting to '' is. I opted to use dangerousStyleValue for consistency with the main path that we set things.
Fixes#14114.
Test Plan:
Verified setting to '' works in Chrome and IE11. (Setting to null works in Chrome but not in IE11.)
The `enableHooks` feature flag used to only control whether the API
was exposed on the React package. But now it also determines if the
dispatcher and implementation are included in the bundle.
We're using hooks in www, so I've switched the feature flag to `true`
in the www build.
(Alternatively, we could have two feature flags: one for the
implementation and dispatcher, and one for exposing the API on the
React package.)
* Avoid double commit by re-rendering immediately and reusing children
To support Suspense outside of concurrent mode, any component that
starts rendering must commit synchronously without being interrupted.
This means normal path, where we unwind the stack and try again from the
nearest Suspense boundary, won't work.
We used to have a special case where we commit the suspended tree in an
incomplete state. Then, in a subsequent commit, we re-render using the
fallback.
The first part — committing an incomplete tree — hasn't changed with
this PR. But I've changed the second part — now we render the fallback
children immediately, within the same commit.
* Add a failing test for remounting fallback in sync mode
* Add failing test for stuck Suspense fallback
* Toggle visibility of Suspense children in mutation phase, not layout
If parent reads visibility of children in a lifecycle, they should have
already updated.
This is required to use lazy.
Test Plan:
* Verified lazy works on a real world use case (shows spinner, shows real content).
* Verified that if I change the primary content's styles to have `display: 'none'` then it never appears (i.e., the code in `unhide` reads the styles successfully)
* Add debug tools package
* Add basic implementation
* Implement inspection of the current state of hooks using the fiber tree
* Support useContext hooks inspection by backtracking from the Fiber
I'm not sure this is safe because the return fibers may not be current
but close enough and it's fast.
We use this to set up the current values of the providers.
* rm copypasta
* Use lastIndexOf
Just in case. I don't know of any scenario where this can happen.
* Support ForwardRef
* Add test for memo and custom hooks
* Support defaultProps resolution
Check for existence of `setTimeout` and `clearTimeout` in the runtime
before using them, to ensure runtimes without them (like .NET ClearScript)
do not crash just by importing `react-dom`.
Mostly to catch this:
```js
useEffect(async () => {
// ...
return cleanup;
});
```
Is this too restrictive? Not sure if you would want to do like
```js
useEffect(() => ref.current.style.color = 'red');
```
which would give a false positive here. We can always relax it to only warn on Promises if people complain.
* Simplify imports in ReactChildFiber
* Import type first in ReactCurrentFiber
* Simplify imports in ReactFiberBeginWork
* Simplify imports in ReactFiberScheduler
* Simplify import in ReactFiberTreeReflection
* Simplify import in ReactFiberUnwindWork
* Remove repeated import
* Fix imports from ReactFiberExpirationTime
* Master imports in ReactFiberBeginWork
Removes the `enableDispatchCallback` feature flag and deletes the
associated code. An earlier version of the Hooks proposal included this
feature but we've since decided to remove it.
Sebastian Markbage
Sebastian Markbåge
Brian Vaughn
Heaven
Bartosz Gordon
Alex Taylor
Andrew Clark
Nathan Schloss
Minh Nguyen
Dominic Gannaway
locknono
Nadav Kaner
Dan Abramov
Sophie Alpert
Tiago Nunes
Marcelo Jorge Vieira
einarq
Simen Bekkhus
Mateusz Burzyński
Chris Bianca
Maksim Markelov
Jordan Harband