Add column number for `viewSourceLineFunction` and renamed the function to `viewUrlSourceFunction` to match the other source function naming conventions
This PR fixes a bug where we would add a transition to the lanes map every time an update occurs. However, we didn't factor in that there might be multiple updates in a transition, which would cause the transition to be added multiple times to the transitionLanes map.
This changes the transitionLanes object from an Array of Arrays to an Array of Sets so that we only add a transition if it hasn't been added before, avoiding duplicates
* [DevTools] front-end for profiling event stack
Adds a side-bar to the profiling tab. Users can now select an update event, and are
shown the callstack from the originating component. When a source path is available
there is now UI to jump to source.
Add FB enabled feature flag: enableProfilerComponentTree for the side-bar.
resolves#24170
This PR cleans up the DevTools codebase by:
* Consolidating `normalizeCodeLocInfo` into one place
* Remove unused source argument in the DevTools component stacks code
This PR adds a component stack field to the `schedule-state-update` event. The algorithm is as follows:
* During profiling, whenever a state update happens collect the parents of the fiber that caused the state update and store it in a map
* After profiling finishes, post process the `schedule-state-update` event and using the parent fibers, generate the component stack by using`describeFiber`, a function that uses error throwing to get the location of the component by calling the component without props.
---
Co-authored-by: Blake Friedman <blake.friedman@gmail.com>
Add aborting to the Flight Server. This encodes the reason as an "error"
row that gets thrown client side. These are still exposed in prod which
is a follow up we'll still have to do to encode them as digests instead.
The error is encoded once and then referenced by each row that needs to
be updated.
In Fizz this got split into Task and Segment. We don't have a concept of
Segment in Flight yet because we don't inline multiple segments into one
"Row". We just emit one "Row" for each Segment if something suspends.
This makes Flight non-deterministic atm but that's something we'll want to
address.
Regardless, right now, this is more like a Task than a Segment.
Before this change we weren't calling onError nor onFatalError if you abort
before completing the shell.
This means that the render never completes and hangs.
Aborting early can happen before even creating the stream for AbortSignal,
before rendering starts in Node since there's an setImmediate atm, or
during rendering.
We can think of transitions on the root as a bunch of tracing markers. Therefore, we can map each transition to a map of pending suspense boundaries. When a transition's pending suspense boundary map is empty, we know that it's complete. This PR:
* Combines the `pendingSuspenseBoundaries` and `transitions` into one `incompleteTransitions` object. This object is a map from a `transition` to a map of `pendingSuspenseBoundaries`
* Refactored code to make it so that every transition has its own `pendingSuspenseBoundaries` map rather than sharing just one.
* Moves the transition complete callback to the root. Alternatively, we can also keep a map of pendingSuspenseBoundaries to transitions on the Offscreen marker, but it's simpler to just call the transition complete callback on the root instead. We also only do this if there are transitions pending, so it shouldn't make too big of a difference
* [DevTools] fix useDeferredValue to match reconciler change
* fixup
* update test to catch original issue
* fix lint
* add safer tests for other composite hooks
This reverts commit 327e4a1f96.
Turns out we hadn't rolled this out internally yet — I mistook
enableClientRenderFallbackOnHydrationMismatch for
said enableClientRenderFallbackOnTextMismatch. Need to revert
until we finish rolling out the change.
* [FORKED] Bugfix: Offscreen instance is null during setState
During a setState, we traverse up the return path and check if any
parent Offscreen components are currently hidden. To do that, we must
access the Offscreen fiber's `stateNode` field.
On a mounted Offscreen fiber, the `stateNode` is never null, so usually
we don't need to refine the type. When a fiber is unmounted, though,
we null out its `stateNode` field to prevent memory cycles. However,
we also null out its `return` field, so I had assumed that an unmounted
Offscreen fiber would never be reachable.
What I didn't consider is that it's possible to call `setState` on a
fiber that never finished mounting. Because it never mounted, it was
never deleted. Because it was never deleted, its `return` field was
never disconnected.
This pattern is always accompanied by a warning but we still need to
account for it. There may also be other patterns where an unmounted
Offscreen instance is reachable, too.
The discovery also suggests it may be better for memory
usage if we don't attach the `return` pointer until the commit phase,
though in order to do that we'd need some other way to track the return
pointer during initial render, like on the stack.
The fix is to add a null check before reading the instance
during setState.
* Add previous commit to list of forked revisions
This PR pushes all of a suspense boundary's transitions onto the transition stack when it goes from hidden to visible so we can pass it to any child suspense boundaries or tracing markers.
Creating an Error captures a stack trace which can be somewhat expensive.
We shouldn't do tthat always for every render.
This also ensures that the stack trace is more useful because you can
follow through the Node.js code to see the cause.
ReactDOMFizzServer(Browser/Node)-test file is really meant to just be a
very shallow tests of the Browser/Node specific streaming APIs.
Most of the general streaming tests are in ReactDOMFizzServer-test instead
of testing all streaming related things in each environment.
The legacy renderToString APIs are mostly covered by ReactServerRendering-test
et al for historical reasons.
This PR adds code to add a marker complete callback to the queue. It also adds code to process marker complete callback.
Marker complete callbacks, in addition to the fields that transition complete callbacks need, also have a `markerName` field
When a suspense boundary suspends or commits, we need to notify the corresponding tracing markers so that they know when to log that they've completed. To do this, we add a stack of tracing markers. In the begin phase, we will push the tracing markers onto the stack, and during the complete/unwind phase we will pop the tracing markers off the stack
In a later PR, we will store the active tracing markers on the suspense boundary, and during the commit phase we will process the active tracing markers by adding/removing the boundary as appropriate.
* [FORKED] Check for infinite update loops even if unmounted
The infinite update loop check doesn't need to run if the component
already unmounted, because an update to an unmounted component can't
cause a re-render. But because we used to run the check in this case,
anyway, I found one test in www that happens to "rely on" this behavior
(accidentally). The test is a pretty messy snapshot thing that I have no
interest fixing so to unblock the sync I'm just going to switch this
back to how it was.
* Add previous commit to forked revisions
The eslint-disable-next-line opt out for prod error minification was not properly working. In the build a replacable error was output even though it was not failing the build. This change refactors the code to avoid the erroneous behavior but a fix for the lint may be better
* [Fizz] Support abort reasons
Fizz supports aborting the render but does not currently accept a reason. The various render functions that use Fizz have some automatic and some user-controlled abort semantics that can be useful to communicate with the running program and users about why an Abort happened.
This change implements abort reasons for renderToReadableStream and renderToPipeable stream as well as legacy renderers such as renderToString and related implementations.
For AbortController implementations the reason passed to the abort method is forwarded to Fizz and sent to the onError handler. If no reason is provided the AbortController should construct an AbortError DOMException and as a fallback Fizz will generate a similar error in the absence of a reason
For pipeable streams, an abort function is returned alongside pipe which already accepted a reason. That reason is now forwarded to Fizz and the implementation described above.
For legacy renderers there is no exposed abort functionality but it is used internally and the reasons provided give useful context to, for instance to the fact that Suspense is not supported in renderToString-like renderers
* Add `isHidden` to OffscreenInstance
We need to be able to read whether an offscreen tree is hidden from
an imperative event. We can store this on its OffscreenInstance.
We were already scheduling a commit effect whenever the visibility
changes, in order to toggle the inner effects. So we can reuse that.
* [FORKED] Bugfix: Revealing a hidden update
This fixes a bug I discovered related to revealing a hidden Offscreen
tree. When this happens, we include in that render all the updates that
had previously been deferred — that is, all the updates that would have
already committed if the tree weren't hidden. This is necessary to avoid
tearing with the surrounding contents. (This was the "flickering"
Suspense bug we found a few years ago: #18411.)
The way we do this is by tracking the lanes of the updates that were
deferred by a hidden tree. These are the "base" lanes. Then, in order
to reveal the hidden tree, we process any update that matches one of
those base lanes.
The bug I discovered is that some of these base lanes may include
updates that were not present at the time the tree was hidden. We cannot
flush those updates earlier that the surrounding contents — that, too,
could cause tearing.
The crux of the problem is that we sometimes reuse the same lane for
base updates and for non-base updates. So the lane alone isn't
sufficient to distinguish between these cases. We must track this in
some other way.
The solution I landed upon was to add an extra OffscreenLane bit to any
update that is made to a hidden tree. Then later when we reveal the
tree, we'll know not to treat them as base updates.
The extra OffscreenLane bit is removed as soon as that lane is committed
by the root (markRootFinished) — at that point, it gets
"upgraded" to a base update.
The trickiest part of this algorithm is reliably detecting when an
update is made to a hidden tree. What makes this challenging is when the
update is received during a concurrent event, while a render is already
in progress — it's possible the work-in-progress render is about to
flip the visibility of the tree that's being updated, leading to a race
condition.
To avoid a race condition, we will wait to read the visibility of the
tree until the current render has finished. In other words, this makes
it an atomic operation. Most of this logic was already implemented
in #24663.
Because this bugfix depends on a moderately risky refactor to the update
queue (#24663), it only works in the "new" reconciler fork. We will roll
it out gradually to www before landing in the main fork.
* Add previous commit to list of forked revisions
We don't need to run DevTools regression tests once an hour, and also it makes getting the most recent react build or react devtools build really annoying, so run them once a day instead
* [Fizz] Disallow complex children in <title> elements
<title> Elements in the DOM can only have Text content. In Fizz if more than one text node is emitted an HTML comment node is used as a text separator. Unfortunately because of the content restriction of the DOM representation of the title element this separator is displayed as escaped text which is not what the component author intended.
This commit special cases title handling, primarily to issue warnings if you pass complex children to <title>. At the moment title expects to receive a single child or an array of length 1. In both cases the type of that child must be string or number. If anything more complex is provided a warning will be logged to the console explaining why this is problematic.
There is no runtime behavior change so broken things are still broken (e.g. returning two text nodes which will cause a separator or using Suspense inside title children) but they should at least be accompanied by warnings that are useful.
One edge case that will now warn but won't technically break an application is if you use a Component that returns a single string as a child of title. This is a form of indirection that works but becasue we cannot discriminate between a Component that will follow the rules and one that violates them the warning is issued regardless.
* fixup dev warning conditional logic
* lints
* fix bugs
* extend onRecoverableError API to support errorInfo
errorInfo has been used in Error Boundaries wiht componentDidCatch for a while now. To date this metadata only contained a componentStack. onRecoverableError only receives an error (type mixed) argument and thus providing additional error metadata was not possible without mutating user created mixed objects.
This change modifies rootConcurrentErrors rootRecoverableErrors, and hydrationErrors so all expect CapturedValue types. additionally a new factory function allows the creation of CapturedValues from a value plus a hash and stack.
In general, client derived CapturedValues will be created using the original function which derives a componentStack from a fiber and server originated CapturedValues will be created using with a passed in hash and optional componentStack.
We have a currently unreproducible flaky e2e test. This PR captures snapshots on e2e test failures so we can better debug flaky e2e tests that don't fail locally.
Luna Ruan
Mengdi Chen
Blake Friedman
Josh Story
Andrew Clark
Sebastian Markbåge
Siyabend Ürün
Kerim Büyükakyüz
dan
Mathieu Dutour