* Move renderer `act` to work loop
* Delete `flushSuspenseFallbacksInTests`
This was meant to be a temporary hack to unblock the `act` work, but it
quickly spread throughout our tests.
What it's meant to do is force fallbacks to flush inside `act` even in
Concurrent Mode. It does this by wrapping the `setTimeout` call in a
check to see if it's in an `act` context. If so, it skips the delay and
immediately commits the fallback.
Really this is only meant for our internal React tests that need to
incrementally render. Nobody outside our team (and Relay) needs to do
that, yet. Even if/when we do support that, it may or may not be with
the same `flushAndYield` pattern we use internally.
However, even for our internal purposes, the behavior isn't right
because a really common reason we flush work incrementally is to make
assertions on the "suspended" state, before the fallback has committed.
There's no way to do that from inside `act` with the behavior of this
flag, because it causes the fallback to immediately commit. This has led
us to *not* use `act` in a lot of our tests, or to write code that
doesn't match what would actually happen in a real environment.
What we really want is for the fallbacks to be flushed at the *end` of
the `act` scope. Not within it.
This only affects the noop and test renderer versions of `act`, which
are implemented inside the reconciler. Whereas `ReactTestUtils.act` is
implemented in "userspace" for backwards compatibility. This is fine
because we didn't have any DOM Suspense tests that relied on this flag;
they all use test renderer or noop.
In the future, we'll probably want to move always use the reconciler
implementation of `act`. It will not affect the prod bundle, because we
currently only plan to support `act` in dev. Though we still haven't
completely figured that out. However, regardless of whether we support a
production `act` for users, we'll still need to write internal React
tests in production mode. For that use case, we'll likely add our own
internal version of `act` that assumes a mock Scheduler and might rely
on hacks that don't 100% align up with the public one.
* Migrate conditional tests to gate pragma
I searched through the codebase for this pattern:
```js
describe('test suite', () => {
if (!__EXPERIMENTAL__) { // or some other condition
test("empty test so Jest doesn't complain", () => {});
return;
}
// Unless we're in experimental mode, none of the tests in this block
// will run.
})
```
and converted them to the `@gate` pragma instead.
The reason this pattern isn't preferred is because you end up disabling
more tests than you need to.
* Add flag for www release channels
Using a heuristic where I check a flag that is known to only be enabled
in www. I left a TODO to instead set the release channel explicitly in
each test config.
* Add more edge cases to fixture
Also adjust some expectations. I think the column should ideally be 1 but varies.
The Example row is one line off because it throws on the hook but should ideally be the component.
Similarly class components with constructors may have the line in the constructor.
* Account for the construct call taking a stack frame
We do this by first searching for the first different frame, then find
the same frames and then find the first different frame again.
* Throw controls
Otherwise they don't get a stack frame associated with them in IE.
* Protect against generating stacks failing
Errors while generating stacks will bubble to the root. Since this technique
is a bit sketchy, we should probably protect against it.
* Don't construct the thing that throws
Instead, we pass the prototype as the "this". It's new every time anyway.
* Implement component stack extraction hack
* Normalize errors in tests
This drops the requirement to include owner to pass the test.
* Special case tests
* Add destructuring to force toObject which throws before the side-effects
This ensures that we don't double call yieldValue or advanceTime in tests.
Ideally we could use empty destructuring but ES lint doesn't like it.
* Cache the result in DEV
In DEV it's somewhat likely that we'll see many logs that add component
stacks. This could be slow so we cache the results of previous components.
* Fixture
* Add Reflect to lint
* Log if out of range.
* Fix special case when the function call throws in V8
In V8 we need to ignore the first line. Normally we would never get there
because the stacks would differ before that, but the stacks are the same if
we end up throwing at the same place as the control.
* Remove unnecessary workInProgress line
* Mutate workInProgress instead of returning
We were ambivalent about this before.
* Make handleError a void method too
We currently use the expiration time to represent the timeout of a
transition. Since we intend to stop treating work priority as a
timeline, we can no longer use this trick.
In this commit, I've changed it to store the event time on the update
object instead. Long term, we will store event time on the root as a map
of transition -> event time. I'm only storing it on the update object
as a temporary workaround to unblock the rest of the changes.
We can't patch the row. We could give these their own "built-in" stack
frame since they're conceptually HoCs. However, from a debugging
perspective this is not very useful meta data and quite noisy. So I'm
just going to exclude them.
All changes in this commit were generated by the following commands.
Copy each module that ends with `.old` to a new file that ends
with `.new`:
```sh
for f in packages/react-reconciler/src/*.old.js; do cp "$f" "$(echo "$f" | sed s/\.old/\.new/)"; done
```
Then transform the internal imports:
```sh
grep -rl --include="*.new.js" '.old' packages/react-reconciler/src/| xargs sed -i '' "s/\.old\'/\.new\'/g"
```
Some of our internal reconciler types have leaked into other packages.
Usually, these types are treated as opaque; we don't read and write
to its fields. This is good.
However, the type is often passed back to a reconciler method. For
example, React DOM creates a FiberRoot with `createContainer`, then
passes that root to `updateContainer`. It doesn't do anything with the
root except pass it through, but because `updateContainer` expects a
full FiberRoot, React DOM is still coupled to all its fields.
I don't know if there's an idiomatic way to handle this in Flow. Opaque
types are simlar, but those only work within a single file. AFAIK,
there's no way to use a package as the boundary for opaqueness.
The immediate problem this presents is that the reconciler refactor will
involve changes to our internal data structures. I don't want to have to
fork every single package that happens to pass through a Fiber or
FiberRoot, or access any one of its fields. So my current plan is to
share the same Flow type across both forks. The shared type will be a
superset of each implementation's type, e.g. Fiber will have both an
`expirationTime` field and a `lanes` field. The implementations will
diverge, but not the types.
To do this, I lifted the type definitions into a separate module.
* Drop the .internal.js suffix on some files that don't need it anymore
* Port some ops patterns to scheduler yield
* Fix triangle test to avoid side-effects in constructor
* Move replaying of setState updaters until after the effect
Otherwise any warnings get silenced if they're deduped.
* Drop .internal.js in more files
* Don't check propTypes on a simple memo component unless it's lazy
Comparing the elementType doesn't work for this because it will never be
the same for a simple element.
This caused us to double validate these. This was covered up because in
internal tests this was deduped since they shared the prop types cache
but since we now inline it, it doesn't get deduped.
* Don't use closures in DevTools injection
Nested closures are tricky. They're not super efficient and when they share
scope between multiple closures they're hard for a compiler to optimize.
It's also unclear how many versions will be created.
By hoisting things out an just make it simple calls the compiler can do
a much better job.
* Store injected hook to work around fast refresh
* Disable console log during the second rerender
* Use the disabled log to avoid double yielding values in scheduler mock
* Reenable debugRenderPhaseSideEffectsForStrictMode in tests that can
* Add failing tests for lazy components
* Fix bailout broken in lazy components due to default props resolving
We should never compare unresolved props with resolved props. Since comparing
resolved props by reference doesn't make sense, we use unresolved props in that
case. Otherwise, resolved props are used.
* Avoid reassigning props warning when we bailout
* Bugfix: Render phase update leads to dropped work
Render phase updates should not affect the `fiber.expirationTime` field.
We don't have to set anything on the fiber because we're going to
process the render phase update immediately.
We also shouldn't reset the `expirationTime` field in between render
passes because it represents the remaining work left in the update
queues. During the re-render, the updates that were skipped in the
original pass are not processed again.
I think my original motivation for using this field for render phase
updates was so I didn't have to add another module level variable.
* Add repro case for #18486
Co-authored-by: Dan Abramov <dan.abramov@me.com>
* 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.
* Remove unnecessary CapturedError fields.
componentName is not necessary and is misleading when the error is caused
elsewhere in the stack. The stack is sufficient.
The many error boundary fields are unnecessary because they can be inferred
by the boundary itself.
* Don't attempt to build a stack twice
If it was possible, it would've been done in createCapturedValue.
* Push the work needed by the works into the forks
This avoids needing this in the npm published case.
This is a variant of the fix in 5a0f1d. We can't rely on the primary
fiber's `childExpirationTime` field to be correct.
In this case, we can read from the Suspense boundary fiber instead.
This will include updates that exist in the fallback fiber, but that's
not a big deal; the important thing is that we don't drop updates.
* 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
* test(SuspenseList): Add failing test for class component
* Reset stateNode when resetWorkInProgress
This is supposed to put the Fiber into the same state as if it was just
created by child fiber reconciliation. For newly created fibers, that means
that stateNode is null.
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
* Revert "ReactDOM.useEvent: enable on internal www and add inspection test (#18395)"
This reverts commit e0ab1a429d.
* Revert "ReactDOM.useEvent: Add support for experimental scopes API (#18375)"
This reverts commit a16b349745.
* ReactDOM.useEvent: Add support for experimental scopes API
* Bugfix: Suspended update must finish to unhide
When we commit a fallback, we cannot unhide the content without including
the level that originally suspended. That's because the work at level
outside the boundary (i.e. everything that wasn't hidden during that
render) already committed.
* Test unblocking with a high-pri update
We store an effect pointer so we can backtrack in the effect list in some
cases. This is a stateful variable. If we interrupt a render we need to
reset it.
This field was added after the optimization was added and I didn't remember
to reset it here.
Otherwise we end up not resetting the firstEffect so it points to a stale
list. As a result children don't end up inserted like we think they were.
Then we try to remove them it errors.
It would be nicer to just get rid of the effect list and use the tree for
effects instead. Maybe we still need something for deletions tho.
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.
This assignment should have been deleted in #18384. It was deleted in
the other branches, but I missed this one. About to open a PR that
includes a test that covers this branch.
* Minor test refactor: `resolveText`
Adds a `resolveText` method as an alternative to using timers. Also
removes dependency on react-cache (for just this one test file; can do
the others later).
Timer option is still there if you provide a `ms` prop.
* Bugfix: Dropped updates in suspended tree
When there are multiple updates at different priority levels inside
a suspended subtree, all but the highest priority one is dropped after
the highest one suspends.
We do have tests that cover this for updates that originate outside of
the Suspense boundary, but not for updates that originate inside.
I'm surprised it's taken us this long to find this issue, but it makes
sense in that transition updates usually originate outside the boundary
or "seam" of the part of the UI that is transitioning.
* Bugfix: Suspense fragment skipped by setState
Fixes a bug where updates inside a suspended tree are dropped because
the fragment fiber we insert to wrap the hidden children is not part of
the return path, so it doesn't get marked during setState.
As a workaround, I recompute `childExpirationTime` right before deciding
to bail out by bubbling it up from the next level of children.
This is something we should consider addressing when we refactor the
Fiber data structure.
* Add back `lastPendingTime` field
This reverts commit 9a541139df.
I want to use this so we can check if there might be any lower priority
updates in a suspended tree.
We can remove it again during the expiration times refactor.
* Use `lastPendingTime` instead of Idle
We don't currently have an mechanism to check if there are lower
priority updates in a subtree, but we can check if there are any in the
whole root. This still isn't perfect but it's better than using Idle,
which frequently leads to redundant re-renders.
When we refactor `expirationTime` to be a bitmask, this will no longer
be necessary because we'll know exactly which "task bits" remain.
* Add a test for updating the fallback
Andrew Clark
Sebastian Markbåge
Dan Abramov
Dominic Gannaway
jddxf
Luna RuanandAndrew Clark
Brian Vaughn
Ricky