Commit Graph

264 Commits

Author SHA1 Message Date
kassens 34cf2eff9f Revert "Revert "[Float] Suspend unstyled content for up to 1 minute (#26532)""
This reverts commit 314f7d3a3c.

DiffTrain build for [7eca7179173c91d3a3501140df0afd45886d1769](https://github.com/facebook/react/commit/7eca7179173c91d3a3501140df0afd45886d1769)
2023-04-19 23:16:43 +00:00
kassens d05978bc8f Revert "Revert "Allow transitions to interrupt Suspensey commits (#26531)""
This reverts commit cfd9d8c73f.

DiffTrain build for [f9231793c57da481cc2030b77f9806cd4dc156f0](https://github.com/facebook/react/commit/f9231793c57da481cc2030b77f9806cd4dc156f0)
2023-04-19 21:40:43 +00:00
kassens 4771ede2e9 Revert "Revert "act: Move didScheduleLegacyUpdate to ensureRootIsScheduled (#26552)""
This reverts commit 7b6e17c8bb.

DiffTrain build for [d08b6cf47b8b4c5bcf8e393fe7b42a896be1b44c](https://github.com/facebook/react/commit/d08b6cf47b8b4c5bcf8e393fe7b42a896be1b44c)
2023-04-19 19:33:44 +00:00
Jan Kassens 6503387506 Update REVISION again 2023-04-19 14:16:38 -04:00
Jan Kassens 33386c26c4 Update REVISION 2023-04-19 14:16:36 -04:00
Jan Kassens 0cb176e5f3 Update REVISION 2023-04-19 13:59:08 -04:00
kassens 9cd51d0053 [actions] commit from special branches iff they exist (#26673)
This creates 2 special branches. If these special branches exist, we'll
commit build artifacts from these branches, main otherwise.

DiffTrain build for [70bda41a01e3d1b785f658ea069f6c505e684686](https://github.com/facebook/react/commit/70bda41a01e3d1b785f658ea069f6c505e684686)
2023-04-19 16:21:16 +00:00
sebmarkbage 80fdf65a7f Switching checked to null should leave the current value (#26667)
I accidentally made a behavior change in the refactor. It turns out that
when switching off `checked` to an uncontrolled component, we used to
revert to the concept of "initialChecked" which used to be stored on
state.

When there's a diff to this computed prop and the value of props.checked
is null, then we end up in a case where it sets `checked` to
`initialChecked`:

https://github.com/facebook/react/blob/5cbe6258bc436b1683080a6d978c27849f1d9a22/packages/react-dom-bindings/src/client/ReactDOMInput.js#L69

Since we never changed `initialChecked` and it's not relevant if
non-null `checked` changes value, the only way this "change" could
trigger was if we move from having `checked` to having null.

This wasn't really consistent with how `value` works, where we instead
leave the current value in place regardless. So this is a "bug fix" that
changes `checked` to be consistent with `value` and just leave the
current value in place. This case should already have a warning in it
regardless since it's going from controlled to uncontrolled.

Related to that, there was also another issue observed in
https://github.com/facebook/react/pull/26596#discussion_r1162295872 and
https://github.com/facebook/react/pull/26588

We need to atomically apply mutations on radio buttons. I fixed this by
setting the name to empty before doing mutations to value/checked/type
in updateInput, and then set the name to whatever it should be. Setting
the name is what ends up atomically applying the changes.

---------

Co-authored-by: Sophie Alpert <git@sophiebits.com>

DiffTrain build for [1f248bdd71](https://github.com/facebook/react/commit/1f248bdd7199979b050e4040ceecfe72dd977fd1)
2023-04-19 15:51:39 +00:00
sophiebits 5aa773a9d9 [Flight] Serialize Date (#26622)
This is kind of annoying because Date implements toJSON so
JSON.stringify turns it into a string before calling our replacer
function.

DiffTrain build for [c6db19f9cd](https://github.com/facebook/react/commit/c6db19f9cdec34bca3625a483a2f85181193b885)
2023-04-19 03:56:59 +00:00
sophiebits db190e0705 Fix input tracking bug (#26627)
In
https://github.com/facebook/react/pull/26573/commits/2019ddc75f448292ffa6429d7625514af192631b,
we changed to set .defaultValue before .value on updates. In some cases,
setting .defaultValue causes .value to change, and since we only set
.value if it has the wrong value, this resulted in us not assigning to
.value, which resulted in inputValueTracking not knowing the right
value. See new test added.

My fix here is to (a) move the value setting back up first and (b)
narrowing the fix in the aforementioned PR to newly remove the value
attribute only if it defaultValue was previously present in props.

The second half is necessary because for types where the value property
and attribute are indelibly linked (hidden checkbox radio submit image
reset button, i.e. spec modes default or default/on from
https://html.spec.whatwg.org/multipage/input.html#dom-input-value-default),
we can't remove the value attribute after setting .value, because that
will undo the assignment we just did! That is, not having (b) makes all
of those types fail to handle updating props.value.

This code is incredibly hard to think about but I think this is right
(or at least, as right as the old code was) because we set .value here
only if the nextProps.value != null, and we now remove defaultValue only
if lastProps.defaultValue != null. These can't happen at the same time
because we have long warned if value and defaultValue are simultaneously
specified, and also if a component switches between controlled and
uncontrolled.

Also, it fixes the test in https://github.com/facebook/react/pull/26626.

DiffTrain build for [b433c379d5](https://github.com/facebook/react/commit/b433c379d55d9684945217c7d375de1082a1abb8)
2023-04-18 17:54:42 +00:00
sophiebits b679adf497 [Flight] Fix style nit from #26623 (#26629)
Maybe this is faster.
https://github.com/facebook/react/pull/26623#discussion_r1167053174

DiffTrain build for [2bfe4b246f](https://github.com/facebook/react/commit/2bfe4b246f58d1f8d357f984fba9a8aa1fa79c73)
2023-04-14 16:54:36 +00:00
sophiebits 6f02a016b0 [Flight] Serialize weird numbers (#26623)
DiffTrain build for [ab2385fa38](https://github.com/facebook/react/commit/ab2385fa388f3d4ebc0b1a9f5a747c82e4aa82ed)
2023-04-14 16:33:48 +00:00
acdlite cda4f89f40 Synchronously flush the transition lane scheduled in a popstate event (#26025)
<!--
  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

Browsers restore state like forms and scroll position right after the
popstate event. To make sure the page work as expected on back or
forward button, we need to flush transitions scheduled in a popstate
synchronously, and only yields if it suspends.
This PR adds a new HostConfig method to check if `window.event ===
'popstate'`, and `scheduleMicrotask` if a transition is scheduled in a
`PopStateEvent`.

## How did you test this change?

yarn test

DiffTrain build for [d121c67004](https://github.com/facebook/react/commit/d121c67004a2e6b0bb5d341843663ef213f64863)
2023-04-13 19:26:30 +00:00
acdlite 94c515f879 Remove revertRemovalOfSiblingPrerendering killswitch (#26549)
removal of sibling prerendering has been rolled out at Meta. We can
delete the flag now.

DiffTrain build for [7b0642bb98](https://github.com/facebook/react/commit/7b0642bb989ec659c6c9891ea16daa0420caab4d)
2023-04-13 01:10:13 +00:00
acdlite 62dabe5299 Throttle retries even if everything has loaded (#26611)
If a Suspense fallback is shown, and the data finishes loading really
quickly after that, we throttle the content from appearing for 500ms to
reduce thrash.

This already works for successive fallback states (like if one fallback
is nested inside another) but it wasn't being applied to the final step
in the sequence: if there were no more unresolved Suspense boundaries in
the tree, the content would appear immediately.

This fixes the throttling behavior so that it applies to all renders
that are the result of suspended data being loaded. (Our internal jargon
term for this is a "retry".)

DiffTrain build for [8256781fdf](https://github.com/facebook/react/commit/8256781fdf3ae1947a7f27ddc78ae11b9989c2cd)
2023-04-13 00:30:30 +00:00
sebmarkbage bdb1083dfa Use already extracted values instead of reading off props for controlled components (#26596)
Since `props.x` is a possibly megamorphic access, it can be slow to
access and trigger recompilation.

When we are looping over the props and pattern matching every key,
anyway, we've already done this work. We can just reuse the same value
by stashing it outside the loop in the stack.

This only makes sense for updates in diffInCommitPhase since otherwise
we don't have the full set of props in that loop.

We also have to be careful not to skip over equal values since we need
to extract them anyway.

DiffTrain build for [6b90976bc1](https://github.com/facebook/react/commit/6b90976bc10f325146b193286435a4b5015ef605)
2023-04-11 17:37:45 +00:00
sebmarkbage fa65896ebf Remove initOption special case (#26595)
This traces back to https://github.com/facebook/react/pull/6449 and then
another before that.

I think that back then we favored the property over the attribute, and
setting the property wouldn't be enough. However, the default path for
these are now using attributes if we don't special case it. So we don't
need it.

The only difference is that we currently have a divergence for
symbol/function behavior between controlled values that use the
getToStringValue helpers which treat them as empty string, where as
everywhere else they're treated as null/missing.

Since this comes with a warning and is a weird error case, it's probably
fine to change.

DiffTrain build for [343a45ffa4](https://github.com/facebook/react/commit/343a45ffa48065e60699bbe68f82d7b62fa02840)
2023-04-11 16:44:01 +00:00
acdlite e2c8a46604 Delete unused eventTimes Fiber field (#26599)
DiffTrain build for [58742c21b8](https://github.com/facebook/react/commit/58742c21b8c3237e8b66c7df4e200504846a01ae)
2023-04-11 12:28:04 +00:00
acdlite be590ddd8f Remove JND delay for non-transition updates (#26597)
Updates that are marked as part of a transition are allowed to block a
render from committing. Generally, other updates cannot — however,
there's one exception that's leftover from a previous iteration of our
Suspense architecture. If an update is not the result of a known urgent
event type — known as "Default" updates — then we allow it to suspend
briefly, as long as the delay is short enough that the user won't
notice. We refer to this delay as a "Just Noticable Difference" (JND)
delay. To illustrate, if the user has already waited 400ms for an update
to be reflected on the screen, the theory is that they won't notice if
you wait an additional 100ms. So React can suspend for a bit longer in
case more data comes in. The longer the user has already waited, the
longer the JND.

While we still believe this theory is sound from a UX perspective, we no
longer think the implementation complexity is worth it. The main thing
that's changed is how we handle Default updates. We used to render
Default updates concurrently (i.e. they were time sliced, and were
scheduled with postTask), but now they are blocking. Soon, they will
also be scheduled with rAF, too, which means by the end of the next rAF,
they will have either finished rendering or the main thread will be
blocked until they do. There are various motivations for this but part
of the rationale is that anything that can be made non-blocking should
be marked as a Transition, anyway, so it's not worth adding
implementation complexity to Default.

This commit removes the JND delay for Default updates. They will now
commit immediately once the render phase is complete, even if a
component suspends.

DiffTrain build for [0b931f90e8](https://github.com/facebook/react/commit/0b931f90e8964183f08ac328e7350d847abb08f9)
2023-04-11 04:24:45 +00:00
sebmarkbage d7b570e955 Move validation of text nesting into ReactDOMComponent (#26594)
Extract validateTextNesting from validateDOMNesting. We only need the
parent tag when validating text nodes. Then validate it in setProp.

DiffTrain build for [ac43bf6870](https://github.com/facebook/react/commit/ac43bf6870a15566507477a4504f22160835c8d3)
2023-04-11 01:48:08 +00:00
sebmarkbage 8ae1b02343 Diff properties in the commit phase instead of generating an update payload (#26583)
This removes the concept of `prepareUpdate()`, behind a flag.

React Native already does everything in the commit phase, but generates
a temporary update payload before applying it.

React Fabric does it both in the render phase. Now it just moves it to a
single host config.

For DOM I forked updateProperties into one that does diffing and
updating in one pass vs just applying a pre-diffed updatePayload.

There are a few downsides of this approach:

- If only "children" has changed, we end up scheduling an update to be
done in the commit phase. Since we traverse through it anyway, it's
probably not much extra.
- It does more work in the commit phase so for a large tree that is
mostly unchanged, it'll stall longer.
- It does some extra work for special cases since that work happens if
anything has changed. We no longer have a deep bailout.
- The special cases now have to each replicate the "clean up old props"
loop, leading to extra code.

The benefit is that this doesn't allocate temporary extra objects
(possibly multiple per element if the array has to resize). It's less
work overall. It also gives us an option to reuse this function for a
sync render optimization.

Another benefit is that if we do the loop in the commit phase I can do
further optimizations by reading all props that I need for special cases
in that loop instead of polymorphic reads from props. This is what I'd
like to do in future refactors that would be stacked on top of this
change.

DiffTrain build for [ca41adb8c1](https://github.com/facebook/react/commit/ca41adb8c1b256705f73d1fb657421a03dfad82c)
2023-04-10 23:16:37 +00:00
gnoff 2e48109083 Rename ReactServerFormatConfig to ReactFizzConfig (#26591)
part of https://github.com/facebook/react/pull/26571

merging separately to improve tracking of file renames

DiffTrain build for [ffb8eaca59](https://github.com/facebook/react/commit/ffb8eaca5966fc7733bd0a23f4055e26d2cc59d7)
2023-04-10 21:59:22 +00:00
gnoff 6e3e2fd44e Implements wiring for Flight to have it's own "HostConfig" (#26590)
Part of https://github.com/facebook/react/pull/26571

Implements wiring for Flight to have it's own "HostConfig" from Fizz.

Historically the ServerFormatConfigs were supposed to be generic enough
to be used by Fizz and Flight. However with the addition of features
like Float the configs have evolved to be more specific to the renderer.
We may want to get back to a place where there is a pure FormatConfig
which can be shared but for now we are embracing the fact that these
runtimes need very different things and DCE cannot adequately remove the
unused stuff for Fizz when pulling this dep into Flight so we are going
to fork the configs and just maintain separate ones.

At first the Flight config will be almost empty but once Float support
in Flight lands it will have a more complex implementation

Additionally this commit normalizes the component files which make up
FlightServerConfig and FlightClientConfig. Now each file that
participates starts with ReactFlightServerConfig... and
ReactFlightClientConfig...

DiffTrain build for [f4f873f628](https://github.com/facebook/react/commit/f4f873f6282e6f2e584990c00fb2aae86db85a8b)
2023-04-10 21:57:51 +00:00
acdlite c608edabba act: Move didScheduleLegacyUpdate to ensureRootIsScheduled (#26552)
`act` uses the `didScheduleLegacyUpdate` field to simulate the behavior
of batching in React <17 and below. It's a quirk leftover from a
previous implementation, not intentionally designed.

This sets `didScheduleLegacyUpdate` every time a legacy root receives an
update as opposed to only when the `executionContext` is empty. There's
no real reason to do it this way over some other way except that it's
how it used to work before #26512 and we should try our best to maintain
the existing behavior, quirks and all, since existing tests may have
come to accidentally rely on it.

This should fix some (though not all) of the internal Meta tests that
started failing after #26512 landed.

Will add a regression test before merging.

DiffTrain build for [fec97ecbc4](https://github.com/facebook/react/commit/fec97ecbc4bc2e0e1407160289a8f5fac5241cbc)
2023-04-10 18:46:20 +00:00
sebmarkbage 75825ffc27 Don't update textarea defaultValue and input checked unnecessarily (#26580)
In #26573 I changed it so that textareas get their defaultValue reset if
you don't specify one.

However, the way that was implemented, it always set it for any update
even if it hasn't changed.

We have a test for that, but that test only works if no properties
update at all so that no update was scheduled. This fixes the test so
that it updates some unrelated prop.

I also found a test for `checked` that needed a similar fix.

Interestingly, we don't do this deduping for `defaultValue` or
`defaultChecked` on inputs and there's no test for that.

DiffTrain build for [9a9da7721e](https://github.com/facebook/react/commit/9a9da7721e5b73a8af242807e463e2af842c58ee)
2023-04-10 02:21:38 +00:00
sebmarkbage bdd6ae36a3 Refactor some controlled component stuff (#26573)
This is mainly renaming some stuff. The behavior change is
hasOwnProperty to nullish check.

I had a bigger refactor that was a dead-end but might as well land this
part and see if I can pick it up later.

DiffTrain build for [e5146cb525](https://github.com/facebook/react/commit/e5146cb5250be1a4e66511af91549859b36ed488)
2023-04-09 22:11:13 +00:00
acdlite b7f5802319 Fix: Move destroy field to shared instance object (#26561)
This fixes the "double free" bug illustrated by the regression test
added in the previous commit.

The underlying issue is that `effect.destroy` field is a mutable field
but we read it during render. This is a concurrency bug — if we had a
borrow checker, it would not allow this.

It's rare in practice today because the field is updated during the
commit phase, which takes a lock on the fiber tree until all the effects
have fired. But it's still theoretically wrong because you can have
multiple Fiber copies each with their own reference to a single destroy
function, and indeed we discovered in production a scenario where this
happens via our current APIs.

In the future these types of scenarios will be much more common because
we will introduce features where effects may run concurrently with the
render phase — i.e. an imperative `hide` method that synchronously hides
a React tree and unmounts all its effects without entering the render
phase, and without interrupting a render phase that's already in
progress.

A future version of React may also be able to run the entire commit
phase concurrently with a subsequent render phase. We can't do this now
because our data structures are not fully thread safe (see: the Fiber
alternate model) but we should be able to do this in the future.

The fix I've introduced in this commit is to move the `destroy` field to
a separate object. The effect "instance" is a shared object that remains
the same for the entire lifetime of an effect. In Rust terms, a RefCell.
The field is `undefined` if the effect is unmounted, or if the effect
ran but is not stateful. We don't explicitly track whether the effect is
mounted or unmounted because that can be inferred by the hiddenness of
the fiber in the tree, i.e. whether there is a hidden Offscreen fiber
above it.

It's unfortunate that this is stored on a separate object, because it
adds more memory per effect instance, but it's conceptually sound. I
think there's likely a better data structure we could use for effects;
perhaps just one array of effect instances per fiber. But I think this
is OK for now despite the additional memory and we can follow up with
performance optimizations later.

---------

Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
Co-authored-by: Rick Hanlon <rickhanlonii@gmail.com>
Co-authored-by: Jan Kassens <jan@kassens.net>

DiffTrain build for [85bb7b685b](https://github.com/facebook/react/commit/85bb7b685b7aec50879703b94dd31523cf69b34d)
2023-04-06 16:13:07 +00:00
sebmarkbage 267d109373 Clean up discrete event replaying (#26558)
We no longer replay and discrete events.

I might re-add a form of this but it'll look a little different.

DiffTrain build for [9cfba0f6ec](https://github.com/facebook/react/commit/9cfba0f6ecc15a342ef184552742c5db6c7f2d3e)
2023-04-05 23:43:08 +00:00
sophiebits 45dac60116 Remove no-fallthrough lint suppressions (#26553)
The lint rule already understands a normal comment. Also a bunch of
these were unnecessary.

DiffTrain build for [790ebc962d](https://github.com/facebook/react/commit/790ebc962de573a37e4755207eba91b07a0fa172)
2023-04-05 03:14:22 +00:00
kassens 82b97bff2d Remove findInstanceBlockingEvent unused parameters (#26534)
## Summary
First three parameters of `findInstanceBlockingEvent` are unused since I
think if we remove the unused parameters it makes it easier to know that
which parameters is need by `findInstanceBlockingEvent`.

## How did you test this change?
Existing tests.

DiffTrain build for [d5fd60f7e6](https://github.com/facebook/react/commit/d5fd60f7e663a5cee61636f8f2dd174efa0fb2f0)
2023-04-04 16:42:09 +00:00
sebmarkbage dc6a835c3c Refactor DOM Bindings Completely Off of DOMProperty Meta Programming (#26546)
There are four places we have special cases based off the DOMProperty
config:

1) DEV-only: ReactDOMUnknownPropertyHook warns for passing booleans to
non-boolean attributes. We just need a simple list of all properties
that are affected by that. We could probably move this in under setProp
instead and have it covered by that list.
2) DEV-only: Hydration. This just needs to read the value from an
attribute and compare it to what we'd expect to see if it was rendered
on the client. This could use some simplification/unification of the
code but I decided to just keep it simple and duplicated since code size
isn't an issue.
3) DOMServerFormatConfig pushAttribute: This just maps the special case
to how to emit it as a HTML attribute.
4) ReactDOMComponent setProp: This just maps the special case to how to
emit it as setAttribute or removeAttribute.

Basically we just have to remember to keep pushAttribute and setProp
aligned. There's only one long switch in prod per environment.

This just turns it all to a giant simple switch statement with string
cases. This is in theory the most optimizable since syntactically all
the information for a hash table is there. However, unfortunately we
know that most VMs don't optimize this very well and instead just turn
them into a bunch of ifs. JSC is best. We can minimize the cost by just
moving common attribute to the beginning of the list.

If we shipped this, maybe VMs will get it together to start optimizing
this case but there's a chicken and egg problem here and the game theory
reality is that we probably don't want to regress. Therefore, I intend
to do a follow up after landing this which reintroduces an object
indirection for simple property aliases. That should be enough to make
the remaining cases palatable. I'll also extract the most common
attributes to the beginning or separate ifs.

Ran attribute-behavior fixture and the table is the same.

DiffTrain build for [eeabb7312f](https://github.com/facebook/react/commit/eeabb7312f509eb2094452a4389646000ea8ea14)
2023-04-04 15:10:50 +00:00
kassens 5b5baac070 Revert "Cleanup enableSyncDefaultUpdate flag (#26236)" (#26528)
This reverts commit b2ae9ddb3b.

While the feature flag is fully rolled out, these tests are also testing
behavior set with an unstable flag on root, which for now we want to
preserve.

Not sure if there's a better way then adding a dynamic feature flag to
the www build?

DiffTrain build for [da94e8b24a](https://github.com/facebook/react/commit/da94e8b24a3f31a3e805f9bf6bba73055aad9d41)
2023-04-04 14:14:05 +00:00
gnoff 8fb86b0ce0 Fix logic around attribute seralization (#26526)
There was a bug in the attribute seralization for stylesheet resources
injected by the Fizz runtime. For boolean properties the attribute value
was set to an empty string but later immediately set to a string coerced
value. This PR fixes that bug and refactors the code paths to be clearer

DiffTrain build for [4a1cc2ddd0](https://github.com/facebook/react/commit/4a1cc2ddd035f5c269e82ab6f7686e2e60d3b3ea)
2023-04-03 16:39:57 +00:00
acdlite 54201b87b5 Fix suspense replaying forward refs (#26535)
Continuation of https://github.com/facebook/react/issues/26420

Fixes https://github.com/facebook/react/issues/26385 and
https://github.com/facebook/react/issues/26419

---------

Co-authored-by: eps1lon <silbermann.sebastian@gmail.com>
Co-authored-by: Andrew Clark <git@andrewclark.io>

DiffTrain build for [7329ea81c1](https://github.com/facebook/react/commit/7329ea81c154d40800e30104be40f050e8c2af3e)
2023-04-02 22:53:13 +00:00
acdlite e1d88e645e [Float] Suspend unstyled content for up to 1 minute (#26532)
We almost never want to show content before its styles have loaded. But
eventually we will give up and allow unstyled content. So this extends
the timeout to a full minute. This somewhat arbitrary — big enough that
you'd only reach it under extreme circumstances.

Note that, like regular Suspense, the app is still interactive while
we're waiting for content to load. Only the unstyled content is blocked
from appearing, not updates in general. A new update will interrupt it.

We should figure out what the browser engines do during initial page
load and consider aligning our behavior with that. It's supposed to be
render blocking by default but there may be some cases where they, too,
give up and FOUC.

DiffTrain build for [0ae348018d](https://github.com/facebook/react/commit/0ae348018d5b3a3f1ccdd92de85d9cc581b2b98d)
2023-03-31 19:50:42 +00:00
acdlite 32a8c3b6d1 Allow transitions to interrupt Suspensey commits (#26531)
I originally made it so that a Suspensey commit — i.e. a commit that's
waiting for a stylesheet, image, or font to load before proceeding —
could not be interrupted by transitions. My reasoning was that Suspensey
commits always time out after a short interval, anyway, so if the
incoming update isn't urgent, it's better to wait to commit the current
frame instead of throwing it away.

I don't think this rationale was correct, for a few reasons. There are
some cases where we'll suspend for a longer duration, like stylesheets —
it's nearly always a bad idea to show content before its styles have
loaded, so we're going to be extend this timeout to be really long.

But even in the case where the timeout is shorter, like fonts, if you
get a new update, it's possible (even likely) that update will allow us
to avoid showing a fallback, like by navigating to a different page. So
we might as well try.

The behavior now matches our behavior for interrupting a suspended
render phase (i.e. `use`), which makes sense because they're not that
conceptually different.

DiffTrain build for [888874673f](https://github.com/facebook/react/commit/888874673f81c08d9c3cfd4a56e2e93fd728894c)
2023-03-31 19:40:52 +00:00
acdlite c1382cc1df Move update scheduling to microtask (#26512)
When React receives new input (via `setState`, a Suspense promise
resolution, and so on), it needs to ensure there's a rendering task
associated with the update. Most of this happens
`ensureRootIsScheduled`.

If a single event contains multiple updates, we end up running the
scheduling code once per update. But this is wasteful because we really
only need to run it once, at the end of the event (or in the case of
flushSync, at the end of the scope function's execution).

So this PR moves the scheduling logic to happen in a microtask instead.
In some cases, we will force it run earlier than that, like for
`flushSync`, but since updates are batched by default, it will almost
always happen in the microtask. Even for discrete updates.

In production, this should have no observable behavior difference. In a
testing environment that uses `act`, this should also not have a
behavior difference because React will push these tasks to an internal
`act` queue.

However, tests that do not use `act` and do not simulate an actual
production environment (like an e2e test) may be affected. For example,
before this change, if a test were to call `setState` outside of `act`
and then immediately call `jest.runAllTimers()`, the update would be
synchronously applied. After this change, that will no longer work
because the rendering task (a timer, in this case) isn't scheduled until
after the microtask queue has run.

I don't expect this to be an issue in practice because most people do
not write their tests this way. They either use `act`, or they write
e2e-style tests.

The biggest exception has been... our own internal test suite. Until
recently, many of our tests were written in a way that accidentally
relied on the updates being scheduled synchronously. Over the past few
weeks, @tyao1 and I have gradually converted the test suite to use a new
set of testing helpers that are resilient to this implementation detail.

(There are also some old Relay tests that were written in the style of
React's internal test suite. Those will need to be fixed, too.)

The larger motivation behind this change, aside from a minor performance
improvement, is we intend to use this new microtask to perform
additional logic that doesn't yet exist. Like inferring the priority of
a custom event.

DiffTrain build for [09c8d25633](https://github.com/facebook/react/commit/09c8d2563300621dc91258a4c2839210e2fbdf0e)
2023-03-31 17:09:18 +00:00
acdlite 04f7a5d391 Clean up enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay (#26521)
This flag is already enabled everywhere except for www, which is blocked
by a few tests that assert on the old behavior. Once www is ready, I'll
land this.

DiffTrain build for [8310854ceb](https://github.com/facebook/react/commit/8310854cebba4d1f404e65ff2064825ee76d78e2)
2023-03-31 14:30:53 +00:00
rickhanlonii 4b5a5b84aa Remove skipUnmountedBoundaries (#26489)
# Overview

Landing this flag internally, will test this PR in React Native before
merging.

DiffTrain build for [ca01f359b9](https://github.com/facebook/react/commit/ca01f359b9236292c749075bb2fd41bb7b569308)
2023-03-31 01:04:01 +00:00
sebmarkbage d9b663c9c3 Limit the meaning of "custom element" to not include is (#26524)
This PR has a bunch of surrounding refactoring. See individual commits.

The main change is that we no longer special case `typeof is ===
'string'` as a special case according to the
`enableCustomElementPropertySupport` flag.

Effectively this means that you can't use custom properties/events,
other than the ones React knows about on `<input is="my-input">`
extensions.

This is unfortunate but there's too many paths that are forked in
inconsistent ways since we fork based on tag name. I think __the
solution is to let all React elements set unknown properties/events in
the same way as this flag__ but that's a bigger change than this flag
implies.

Since `is` is not universally supported yet anyway, this doesn't seem
like a huge loss. Attributes still work.

We still support passing the `is` prop and turn that into the
appropriate createElement call.

@josepharhar

DiffTrain build for [43a70a610d](https://github.com/facebook/react/commit/43a70a610dd2cc298ab5592deebfbf8f7bbac127)
2023-03-30 22:43:52 +00:00
sebmarkbage dd1749c135 Warn for ARIA typos on custom elements (#26523)
Normally we allow any attribute/property on custom elements. However
it's a shared namespace. The `aria-` namespace applies to all generic
elements which are shared with custom elements. So arguably adding
custom extensions there is a really bad idea since it can conflict with
future additions.

It's possible there is a new standard one that's polyfilled by a custom
element but the same issue applies to React in general that we might
warn for very new additions so we just have to be quick on that.

cc @josepharhar

DiffTrain build for [1a1d61fed9](https://github.com/facebook/react/commit/1a1d61fed98a02c9b1bac029d0bc11c3e4db896d)
2023-03-30 20:36:16 +00:00
sebmarkbage b964e2e555 Refactor DOMProperty and CSSProperty (#26513)
This is a step towards getting rid of the meta programming in
DOMProperty and CSSProperty.

This moves isAttributeNameSafe and isUnitlessNumber to a separate shared
modules.

isUnitlessNumber is now a single switch instead of meta-programming.
There is a slight behavior change here in that I hard code a specific
set of vendor-prefixed attributes instead of prefixing all the unitless
properties. I based this list on what getComputedStyle returns in
current browsers. I removed Opera prefixes because they were [removed in
Opera](https://dev.opera.com/blog/css-vendor-prefixes-in-opera-12-50-snapshots/)
itself. I included the ms ones mentioned [in the original
PR](https://github.com/facebook/react/commit/5abcce534382d85887f3d33475e8e54e3b5d8457).
These shouldn't really be used anymore anyway so should be pretty safe.
Worst case, they'll fallback to the other property if you specify both.

Finally I inline the mustUseProperty special cases - which are also the
only thing that uses propertyName. These are really all controlled
components and all booleans.

I'm making a small breaking change here by treating `checked` and
`selected` specially only on the `input` and `option` tags instead of
all tags. That's because those are the only DOM nodes that actually have
those properties but we used to set them as expandos instead of
attributes before. That's why one of the tests is updated to now use
`input` instead of testing an expando on a `div` which isn't a real use
case. Interestingly this also uncovered that we update checked twice for
some reason but keeping that logic for now.

Ideally `multiple` and `muted` should move into `select` and
`audio`/`video` respectively for the same reason.

No change to the attribute-behavior fixture.

DiffTrain build for [73deff0d51](https://github.com/facebook/react/commit/73deff0d5162160c0aafa5cd0b87e11143fe9938)
2023-03-30 18:35:49 +00:00
acdlite b0ce267457 Clean up deferRenderPhaseUpdateToNextBatch (#26511)
This is a change to some undefined behavior that we though we would do
at one point but decided not to roll out. It's already disabled
everywhere, so this just deletes the branch from the implementation and
the tests.

DiffTrain build for [2d51251e60](https://github.com/facebook/react/commit/2d51251e608b7b1a8baf79ae6bdba81ed8e1939a)
2023-03-30 18:16:13 +00:00
sebmarkbage 14a8309c26 Generate safe javascript url instead of throwing with disableJavaScriptURLs is on (#26507)
We currently throw an error when disableJavaScriptURLs is on and trigger
an error boundary. I kind of thought that's what would happen with CSP
or Trusted Types anyway. However, that's not what happens. Instead, in
those environments what happens is that the error is triggered when you
try to actually visit those links. So if you `preventDefault()` or
something it'll never show up and since the error just logs to the
console or to a violation logger, it's effectively a noop to users.

We can simulate the same without CSP by simply generating a different
`javascript:` url that throws instead of executing the potential attack
vector.

This still allows these to be used - at least as long as you
preventDefault before using them in practice. This might be legit for
forms. We still don't recommend using them for links-as-buttons since
it'll be possible to "Open in a New Tab" and other weird artifacts. For
links we still recommend the technique of assigning a button role etc.

It also is a little nicer when an attack actually happens because at
least it doesn't allow an attacker to trigger error boundaries and
effectively deny access to a page.

DiffTrain build for [4c2fc01900](https://github.com/facebook/react/commit/4c2fc01900f50b5b1081a2fb8609ea2668bc05b6)
2023-03-30 03:43:59 +00:00
eps1lon c49cacaf17 [Flight] Add support BigInt support (#26479)
## Summary

Adds support for sending `BigInt` to Flight and Flight Reply

## How did you test this change?

- added tests

DiffTrain build for [fd0511c728](https://github.com/facebook/react/commit/fd0511c728e186905b7f0e71f072b4b247d6f29f)
2023-03-29 16:48:12 +00:00
sebmarkbage fc1026b8b3 Refactor DOM special cases per tags including controlled fields (#26501)
I use a shared helper when setting properties into a helper whether it's
initial or update.

I moved the special cases per tag to commit phase so we can check it
only once. This also effectively inlines getHostProps which can be done
in a single check per prop key.

The diffProperties operation is simplified to mostly just generating a
plain diff of all properties, generating an update payload. This might
generate a few more entries that are now ignored in the commit phase.
that previously would've been ignored earlier. We could skip this and
just do the whole diff in the commit phase by always scheduling a commit
phase update.

I tested the attribute table (one change documented below) and a few
select DOM fixtures.

DiffTrain build for [85de6fde51](https://github.com/facebook/react/commit/85de6fde515148babd36eae2b7384ad8e62b732a)
2023-03-29 02:45:08 +00:00
rickhanlonii 4b35aafb56 Make disableSchedulerTimeoutInWorkLoop a static ff (#26497)
## Overview

There's a known infinite loop with this but we're not running an
experiment any time soon.

DiffTrain build for [f62cb39ee5](https://github.com/facebook/react/commit/f62cb39ee58e5918806218de2be8d2833023e7fc)
2023-03-28 18:17:04 +00:00
rickhanlonii 0dd3c25652 Remove disableNativeComponentFrames (#26490)
## Overview

I'm landing this flag internally so we can delete this

DiffTrain build for [41b4714f19](https://github.com/facebook/react/commit/41b4714f195868193ddfb2a89d2373b1c61387f4)
2023-03-28 13:52:23 +00:00
kassens 2a33a65802 [flow] make Flow suppressions explicit on the error (#26487)
Added an explicit type to all $FlowFixMe suppressions to reduce
over-suppressions of new errors that might be caused on the same lines.

Also removes suppressions that aren't used (e.g. in a `@noflow` file as
they're purely misleading)

Test Plan:
yarn flow-ci

DiffTrain build for [afea1d0c53](https://github.com/facebook/react/commit/afea1d0c536e0336735b0ea5c74f635527b65785)
2023-03-27 11:49:00 +00:00
acdlite 49e6dee1af Suspensily committing a prerendered tree (#26434)
Prerendering a tree (i.e. with Offscreen) should not suspend the commit
phase, because the content is not yet visible. However, when revealing a
prerendered tree, we should suspend the commit phase if resources in the
prerendered tree haven't finished loading yet.

To do this properly, we need to visit all the visible nodes in the tree
that might possibly suspend. This includes nodes in the current tree,
because even though they were already "mounted", the resources might not
have loaded yet, because we didn't suspend when it was prerendered.

We will need to add this capability to the Offscreen component's
"manual" mode, too. Something like a `ready()` method that returns a
promise that resolves when the tree has fully loaded.

Also includes some fixes to #26450. See PR for details.

DiffTrain build for [768f965de2](https://github.com/facebook/react/commit/768f965de2d4c6be7f688562ef02382478c82e5b)
2023-03-27 03:53:29 +00:00