Summary:
Stacked on top of #28498 for test fixes.
### Don't Rethrow
When we started React it was 1:1 setState calls a series of renders and
if they error, it errors where the setState was called. Simple. However,
then batching came and the error actually got thrown somewhere else.
With concurrent mode, it's not even possible to get setState itself to
throw anymore.
In fact, all APIs that can rethrow out of React are executed either at
the root of the scheduler or inside a DOM event handler.
If you throw inside a React.startTransition callback that's sync, then
that will bubble out of the startTransition but if you throw inside an
async callback or a useTransition we now need to handle it at the hook
site. So in 19 we need to make all React.startTransition swallow the
error (and report them to reportError).
The only one remaining that can throw is flushSync but it doesn't really
make sense for it to throw at the callsite neither because batching.
Just because something rendered in this flush doesn't mean it was
rendered due to what was just scheduled and doesn't mean that it should
abort any of the remaining code afterwards. setState is fire and forget.
It's send an instruction elsewhere, it's not part of the current
imperative code.
Error boundaries never rethrow. Since you should really always have
error boundaries, most of the time, it wouldn't rethrow anyway.
Rethrowing also actually currently drops errors on the floor since we
can only rethrow the first error, so to avoid that we'd need to call
reportError anyway. This happens in RN events.
The other issue with rethrowing is that it logs an extra console.error.
Since we're not sure that user code will actually log it anywhere we
still log it too just like we do with errors inside error boundaries
which leads all of these to log twice.
The goal of this PR is to never rethrow out of React instead, errors
outside of error boundaries get logged to reportError. Event system
errors too.
### Breaking Changes
The main thing this affects is testing where you want to inspect the
errors thrown. To make it easier to port, if you're inside `act` we
track the error into act in an aggregate error and then rethrow it at
the root of `act`. Unlike before though, if you flush synchronously
inside of act it'll still continue until the end of act before
rethrowing.
I expect most user code breakages would be to migrate from `flushSync`
to `act` if you assert on throwing.
However, in the React repo we also have `internalAct` and the
`waitForThrow` helpers. Since these have to use public production
implementations we track these using the global onerror or process
uncaughtException. Unlike regular act, includes both event handler
errors and onRecoverableError by default too. Not just render/commit
errors. So I had to account for that in our tests.
We restore logging an extra log for uncaught errors after the main log
with the component stack in it. We use `console.warn`. This is not yet
ignorable if you preventDefault to the main error event. To avoid
confusion if you don't end up logging the error to console I just added
`An error occurred`.
### Polyfill
All browsers we support really supports `reportError` but not all test
and server environments do, so I implemented a polyfill for browser and
node in `shared/reportGlobalError`. I don't love that this is included
in all builds and gets duplicated into isomorphic even though it's not
actually needed in production. Maybe in the future we can require a
polyfill for this.
### Follow Ups
In a follow up, I'll make caught vs uncaught error handling be
configurable too.
---------
DiffTrain build for commit https://github.com/facebook/react/commit/6786563f3cbbc9b16d5a8187207b5bd904386e53.
Changelog:
[Internal]
Reviewed By: kassens
Differential Revision: D55408481
Pulled By: yungsters
fbshipit-source-id: 598aa306369e21cb3e93ad6041a87bfbaa9eef9e
Co-authored-by: Ricky Hanlon <rickhanlonii@gmail.com>
Summary:
Changelog: [Internal]
Along with all the places using it like the `_debugSource` on Fiber.
This still lets them be passed into `createElement` (and JSX dev
runtime) since those can still be used in existing already compiled code
and we don't want that to start spreading to DOM attributes.
We used to have a DEV mode that compiles the source location of JSX into
the compiled output. This was nice because we could get the actual call
site of the JSX (instead of just somewhere in the component). It had a
bunch of issues though:
- It only works with JSX.
- The way this source location is compiled is different in all the
pipelines along the way. It relies on this transform being first and the
source location we want to extract but it doesn't get preserved along
source maps and don't have a way to be connected to the source hosted by
the source maps. Ideally it should just use the mechanism other source
maps use.
- Since it's expensive it only works in DEV so if it's used for
component stacks it would vary between dev and prod.
- It only captures the callsite of the JSX and not the stack between the
component and that callsite. In the happy case it's in the component but
not always.
Instead, we have another zero-cost trick to extract the call site of
each component lazily only if it's needed. This ensures that component
stacks are the same in DEV and PROD. At the cost of worse line number
information.
The better way to get the JSX call site would be to get it from `new
Error()` or `console.createTask()` inside the JSX runtime which can
capture the whole stack in a consistent way with other source mappings.
We might explore that in the future.
This removes source location info from React DevTools and React Native
Inspector. The "jump to source code" feature or inspection can be made
lazy instead by invoking the lazy component stack frame generation. That
way it can be made to work in prod too. The filtering based on file path
is a bit trickier.
When redesigned this UI should ideally also account for more than one
stack frame.
With this change the DEV only Babel transforms are effectively
deprecated since they're not necessary for anything.
DiffTrain build for commit https://github.com/facebook/react/commit/37d901e2b81e12d40df7012c6f8681b8272d2555.
Reviewed By: kassens
Differential Revision: D53543159
Pulled By: tyao1
fbshipit-source-id: 8e5509a16ea8d3234881e2305149326fb31e3845
Summary:
This automatically exposes `$$FORM_ACTIONS` on Server References coming
from Flight. So that when they're used in a form action, we can encode
the ID for the server reference as a hidden field or as part of the name
of a button.
If the Server Action is a bound function it can have complex data
associated with it. In this case this additional data is encoded as
additional form fields.
To process a POST on the server there's now a `decodeAction` helper that
can take one of these progressive posts from FormData and give you a
function that is prebound with the correct closure and FormData so that
you can just invoke it.
I updated the fixture which now has a "Server State" that gets
automatically refreshed. This also lets us visualize form fields.
There's no "Action State" here for showing error messages that are not
thrown, that's still up to user space.
DiffTrain build for commit https://github.com/facebook/react/commit/aef7ce5547c9489dc48e31f69b002cd17206e0cb.
Changelog: [Internal]
<< DO NOT EDIT BELOW THIS LINE >>
Reviewed By: christophpurrer
Differential Revision: D45548613
Pulled By: tyao1
fbshipit-source-id: 0d4206c18c4818fa410e8b18f3a0f0942237c91b
Summary:
E.g. if we suspend (throw a promise) in pushStartInstance today we might
have already pushed some chunks (or even child segments potentially). We
should revert back to where we were.
This doesn't usually happen because when we suspend in a component it
doesn't write anything itself, it'll always defer to som host instance
to do the writing.
There was a todo about this already but I'm not 100% sure it's always
safe when suspending. It should be safe when suspending just regularly
because it's just a noop. We might not even want "throwing a promise" in
this mechanism to be supported longer term but for now that's how a
suspend in internals.
DiffTrain build for commit https://github.com/facebook/react/commit/c10010a6a00911fe99452bc561dd47c3e15f4eb8.
Changelog: [Internal]
<< DO NOT EDIT BELOW THIS LINE >>
Reviewed By: christophpurrer
Differential Revision: D45547661
Pulled By: tyao1
fbshipit-source-id: 69654c95f06bc0f056fc8231e41aa395931af228
Summary:
Usually we don't have to do this since we only set these in the loop but
the ReactCustomFormAction props are optional so they might be undefined.
Also moved it to a general type since it's a semi-public API.
DiffTrain build for commit https://github.com/facebook/react/commit/fa7a447b9ce5a4f0be592fc2946380b0fa3b29c0.
Changelog: [Internal]:
<< DO NOT EDIT BELOW THIS LINE >>
Reviewed By: sammy-SC
Differential Revision: D45537647
Pulled By: tyao1
fbshipit-source-id: 15aa00347467e424e3fa7d509139b2f87cdcdb69
Summary:
Stacked on top of #26735.
This allows a framework to add a `$$FORM_ACTION` property to a function.
This lets the framework return a set of props to use in place of the
function but only during SSR. Effectively, this lets you implement
progressive enhancement of form actions using some other way instead of
relying on the replay feature.
This will be used by RSC on Server References automatically by
convention in a follow up, but this mechanism can also be used by other
frameworks/libraries.
DiffTrain build for commit https://github.com/facebook/react/commit/559e83aebb2026035d47aa0ebf842f78d4cd6757.
Changelog: [Internal]
<< DO NOT EDIT BELOW THIS LINE >>
Reviewed By: sammy-SC
Differential Revision: D45454355
Pulled By: tyao1
fbshipit-source-id: 4b4b65c77ecbc113a79e51e4aff97b2e3b88c31a
Summary:
This allows us to emit extra ephemeral data that will only be used on
server rendered forms.
First I refactored the shouldSkip functions to now just do that work
inside the canHydrate methods. This makes the Config bindings a little
less surface area but it also helps us optimize a bit since we now can
look at the code together and find shared paths.
canHydrate returns the instance if it matches, that used to just be
there to refine the type but it can also be used to just return a
different instance later that we find. If we don't find one, we'll bail
out and error regardless so no need to skip past anything.
DiffTrain build for commit https://github.com/facebook/react/commit/67f4fb02130b1fe1856289e3b66bb0b8cca57ff7.
Changelog: [Internal]
<< DO NOT EDIT BELOW THIS LINE >>
Reviewed By: sammy-SC
Differential Revision: D45453502
Pulled By: tyao1
fbshipit-source-id: 1ac34bcc2d59fdbfbc25e51f6bd5354777a33f96
Summary:
We used to have Event Replaying for any kind of Discrete event where
we'd track any event after hydrateRoot and before the async code/data
has loaded in to hydrate the target. However, this didn't really work
out because code inside event handlers are expected to be able to
synchronously read the state of the world at the time they're invoked.
If we replay discrete events later, the mutable state around them like
selection or form state etc. may have changed.
This limitation doesn't apply to Client Actions:
- They're expected to be async functions that themselves work
asynchronously. They're conceptually also in the "navigation" events
that happen after the "submit" events so they're already not
synchronously even before the first `await`.
- They're expected to operate mostly on the FormData as input which we
can snapshot at the time of the event.
This PR adds a bit of inline script to the Fizz runtime (or external
runtime) to track any early submit events on the page - but only if the
action URL is our placeholder `javascript:` URL. We track a queue of
these on `document.$$reactFormReplay`. Then we replay them in order as
they get hydrated and we get a handle on the Client Action function.
I add the runtime to the `bootstrapScripts` phase in Fizz which is
really technically a little too late, because on a large page, it might
take a while to get to that script even if you have displayed the form.
However, that's also true for external runtime. So there's a very short
window we might miss an event but it's good enough and better than
risking blocking display on this script.
The main thing that makes the replaying difficult to reason about is
that we can have multiple instance of React using this same queue. This
would be very usual but you could have two different Reacts SSR:ing
different parts of the tree and using around the same version. We don't
have any coordinating ids for this. We could stash something on the form
perhaps but given our current structure it's more difficult to get to
the form instance in the commit phase and a naive solution wouldn't
preserve ordering between forms.
This solution isn't 100% guaranteed to preserve ordering between
different React instances neither but should be in order within one
instance which is the common case.
The hard part is that we don't know what instance something will belong
to until it hydrates. So to solve that I keep everything in the original
queue while we wait, so that ordering is preserved until we know which
instance it'll go into. I ended up doing a bunch of clever tricks to
make this work. These could use a lot more tests than I have right now.
Another thing that's tricky is that you can update the action before
it's replayed but we actually want to invoke the old action if that
happens. So we have to extract it even if we can't invoke it right now
just so we get the one that was there during hydration.
DiffTrain build for commit https://github.com/facebook/react/commit/bf449ee74e5e98af3d08c87bb6a9f22a021f3522.
Changelog: [Internal]
Reviewed By: poteto
Differential Revision: D45274088
Pulled By: kassens
fbshipit-source-id: 7e9d96731f0bc72ea5fa3ffc6ce3f4a1dfe80d90
Summary:
We currently use rollup to make an adhoc bundle from the file system
when we're testing an import of an external file.
This doesn't follow all the interception rules that we use in jest and
in our actual builds.
This switches to just using jest require() to load these. This means
that they effectively have to load into the global document so this only
works with global document tests which is all we have now anyway.
DiffTrain build for commit https://github.com/facebook/react/commit/64d6be71224c4241ba8f9d80747d53c0fd6224e7.
Changelog: [Internal]
Reviewed By: poteto
Differential Revision: D45272583
Pulled By: kassens
fbshipit-source-id: bc06743c0997464d0459ecaa1dfefb4d5dc783d0
Summary:
This is consistent with what we used to do but not what we want to do.
DiffTrain build for commit https://github.com/facebook/react/commit/9ece58ebaa46f8b5e90a6ad71be4919e1dc9c563.
Changelog: [Internal]
Reviewed By: poteto
Differential Revision: D45253561
Pulled By: kassens
fbshipit-source-id: c739cff3a13d4ddc0569601a018a53ff5cc21c9b
Summary:
Insert temporary input node to polyfill submitter argument in FormData.
This works for buttons too and fixes a bug where the type attribute
wasn't reset.
I also exclude the submitter if it's a function action. This ensures
that we don't include the generated "name" when the action is a server
action. Conceptually that name doesn't exist.
DiffTrain build for commit https://github.com/facebook/react/commit/5e5342b10059bf90738a8d9171fcf0af9d9d5d51.
Changelog: [Internal]
Reviewed By: kassens
Differential Revision: D45240323
fbshipit-source-id: 3e9a8076942abdc24cf25b9dd9f130727ec3c0fd
Summary:
The Promise as a child case seems buggy. It ends up throwing the Promise
as fatal when used in Sync rendering.
DiffTrain build for commit https://github.com/facebook/react/commit/9c58a0b6475509f9124da578207aa0d3b7364035.
Changelog: [Internal]
Reviewed By: kassens
Differential Revision: D45239545
fbshipit-source-id: 1b377ec87bc3d1bbe07d7fd9f68235629adff764
Summary:
Fizz can emit whatever it wants for the SSR version of these fields when
it's a function action so they might not align with what is in the
previous props. Therefore we need to force them to update if we're
updating to a non-function where they might be relevant again.
DiffTrain build for commit https://github.com/facebook/react/commit/2fa632381839c8732dad9107b90911163b7f2b7a.
Changelog: [Internal]
Reviewed By: poteto
Differential Revision: D45238365
fbshipit-source-id: b75b4779bd3759362d69f96994ba781982f650a0
Summary:
Use the Blob constructor + append with filename instead of File
constructor. Node.js doesn't expose a global File constructor but does
support it in this form.
Queue fields until we get the 'end' event from the previous file. We
rely on previous files being available by the time a field is resolved.
However, since the 'end' event in Readable is fired after two
micro-tasks, these are not resolved in order.
I use a queue of the fields while we're still waiting on files to
finish. This still doesn't resolve files and fields in order relative to
each other but that doesn't matter for our usage.
DiffTrain build for commit https://github.com/facebook/react/commit/a21d1475ffd7225a463f2d0c0c9b732c8dd795eb.
Changelog: [Internal]
Reviewed By: poteto
Differential Revision: D45238341
fbshipit-source-id: d4ce8cf0201a231263c753989267e85f92908b9e
