Commit Graph
111 Commits
Author SHA1 Message Date
Josh Story 36e4cbe2e9 [Float][Flight] Flight support for Float (#26502)
Stacked on #26557 

Supporting Float methods such as ReactDOM.preload() are challenging for
flight because it does not have an easy means to convey direct
executions in other environments. Because the flight wire format is a
JSON-like serialization that is expected to be rendered it currently
only describes renderable elements. We need a way to convey a function
invocation that gets run in the context of the client environment
whether that is Fizz or Fiber.

Fiber is somewhat straightforward because the HostDispatcher is always
active and we can just have the FlightClient dispatch the serialized
directive.

Fizz is much more challenging becaue the dispatcher is always scoped but
the specific request the dispatch belongs to is not readily available.
Environments that support AsyncLocalStorage (or in the future
AsyncContext) we will use this to be able to resolve directives in Fizz
to the appropriate Request. For other environments directives will be
elided. Right now this is pragmatic and non-breaking because all
directives are opportunistic and non-critical. If this changes in the
future we will need to reconsider how widespread support for async
context tracking is.

For Flight, if AsyncLocalStorage is available Float methods can be
called before and after await points and be expected to work. If
AsyncLocalStorage is not available float methods called in the sync
phase of a component render will be captured but anything after an await
point will be a noop. If a float call is dropped in this manner a DEV
warning should help you realize your code may need to be modified.

This PR also introduces a way for resources (Fizz) and hints (Flight) to
flush even if there is not active task being worked on. This will help
when Float methods are called in between async points within a function
execution but the task is blocked on the entire function finishing.

This PR also introduces deduping of Hints in Flight using the same
resource keys used in Fizz. This will help shrink payload sizes when the
same hint is attempted to emit over and over again
2023-04-21 20:45:51 -07:00
Andrew Clark 8f42196892 Change DOM HostContext to number instead of string (#26698)
In React DOM, we use HostContext to represent the namespace of whatever
is currently rendering — SVG, Math, or HTML. Because there is a fixed
set of possible values, we can switch this to be a number instead. My
motivation is that I want to start tracking additional information in
this type, and I want to pack all of it into a single number instead of
turning it into an object. For better performance.

(In dev, the host context type is already an object that includes
additional information, but that's dev so who cares.)

Technically, before this change, the host context could be any namespace
URI string, but any value other than SVG or Math was treated the same
way. Only SVG and Math have special behavior. So in the new structure,
there are three enum values: SVG, Math, or None, which represents the
HTML namespace as well as all other possible namespaces.
2023-04-21 21:44:43 -04:00
Andrew Clark fd3fb8e3c5 Rethrow errors from form actions (#26689)
This is the next step toward full support for async form actions.

Errors thrown inside form actions should cause the form to re-render and
throw the error so it can be captured by an error boundary. The behavior
is the same if the `<form />` had an internal useTransition hook, which
is pretty much exactly how we implement it, too.

The first time an action is called, the form's HostComponent is
"upgraded" to become stateful, by lazily mounting a list of hooks. The
rest of the implementation for function components can be shared.

Because the error handling behavior added in this commit is just using
useTransition under-the-hood, it also handles pending states, too.
However, this pending state can't be observed until we add a new hook
for that purpose. I'll add this next.
2023-04-21 13:29:46 -04:00
Josh Story fdad813ac7 [Float][Fiber] Enable Float methods to be called outside of render (#26557)
Stacked on #26570 

Previously we restricted Float methods to only being callable while
rendering. This allowed us to make associations between calls and their
position in the DOM tree, for instance hoisting preinitialized styles
into a ShadowRoot or an iframe Document.

When considering how we are going to support Flight support in Float
however it became clear that this restriction would lead to compromises
on the implementation because the Flight client does not execute within
the context of a client render. We want to be able to disaptch Float
directives coming from Flight as soon as possible and this requires
being able to call them outside of render.

this patch modifies Float so that its methods are callable anywhere. The
main consequence of this change is Float will always use the Document
the renderer script is running within as the HoistableRoot. This means
if you preinit as style inside a component render targeting a ShadowRoot
the style will load in the ownerDocument not the ShadowRoot. Practially
speaking it means that preinit is not useful inside ShadowRoots and
iframes.

This tradeoff was deemed acceptable because these methods are
optimistic, not critical. Additionally, the other methods, preconntect,
prefetchDNS, and preload, are not impacted because they already operated
at the level of the ownerDocument and really only interface with the
Network cache layer.

I added a couple additional fixes that were necessary for getting tests
to pass that are worth considering separately.

The first commit improves the diff for `waitForThrow` so it compares
strings if possible.

The second commit makes invokeGuardedCallback not use metaprogramming
pattern and swallows any novel errors produced from trying to run the
guarded callback. Swallowing may not be the best we can do but it at
least protects React against rapid failure when something causes the
dispatchEvent to throw.
2023-04-20 14:40:25 -07:00
Sophie Alpert 22d5942675 Add two event system cleanup TODOs (#26678)
There is so much old stuff in these files. I am weeping.
2023-04-19 18:41:46 -07:00
Sophie Alpert 767f52237c Use .slice() for all substring-ing (#26677)
- substr is Annex B
- substring silently flips its arguments if they're in the "wrong order", which is confusing
- slice is better than sliced bread (no pun intended) and also it works the same way on Arrays so there's less to remember

---

> I'd be down to just lint and enforce a single form just for the potential compression savings by using a repeated string.

_Originally posted by @sebmarkbage in https://github.com/facebook/react/pull/26663#discussion_r1170455401_
2023-04-19 14:26:01 -07:00
Sebastian Markbåge c826dc50de Add (Client) Functions as Form Actions (#26674)
This lets you pass a function to `<form action={...}>` or `<button
formAction={...}>` or `<input type="submit formAction={...}>`. This will
behave basically like a `javascript:` URL except not quite implemented
that way. This is a convenience for the `onSubmit={e => {
e.preventDefault(); const fromData = new FormData(e.target); ... }`
pattern.

You can still implement a custom `onSubmit` handler and if it calls
`preventDefault`, it won't invoke the action, just like it would if you
used a full page form navigation or javascript urls. It behaves just
like a navigation and we might implement it with the Navigation API in
the future.

Currently this is just a synchronous function but in a follow up this
will accept async functions, handle pending states and handle errors.

This is implemented by setting `javascript:` URLs, but these only exist
to trigger an error message if something goes wrong instead of
navigating away. Like if you called `stopPropagation` to prevent React
from handling it or if you called `form.submit()` instead of
`form.requestSubmit()` which by-passes the `submit` event. If CSP is
used to ban `javascript:` urls, those will trigger errors when these
URLs are invoked which would be a different error message but it's still
there to notify the user that something went wrong in the plumbing.

Next up is improving the SSR state with action replaying and progressive
enhancement.
2023-04-19 16:31:08 -04:00
Sebastian MarkbågeandSophie Alpert 1f248bdd71 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>
2023-04-19 11:46:29 -04:00
Sophie Alpert b433c379d5 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.
2023-04-18 10:49:32 -07:00
Tianyu Yao d121c67004 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
2023-04-13 15:21:19 -04:00
Sebastian Markbåge 6b90976bc1 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.
2023-04-11 13:32:47 -04:00
Sebastian Markbåge 343a45ffa4 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.
2023-04-11 12:39:00 -04:00
Sebastian Markbåge ac43bf6870 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.
2023-04-10 21:41:53 -04:00
Sebastian Markbåge ca41adb8c1 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.
2023-04-10 19:09:28 -04:00
Josh Story b55d319559 Rename HostConfig files to FiberConfig to clarify they are configs fo… (#26592)
part of https://github.com/facebook/react/pull/26571

merging separately to improve tracking of files renames in git

Rename HostConfig files to FiberConfig to clarify they are configs for
Fiber and not Fizz/Flight. This better conforms to the naming used in
Flight and now Fizz of `ReactFlightServerConfig` and `ReactFizzConfig`
2023-04-10 14:58:44 -07:00
Josh Story ffb8eaca59 Rename ReactServerFormatConfig to ReactFizzConfig (#26591)
part of https://github.com/facebook/react/pull/26571

merging separately to improve tracking of file renames
2023-04-10 14:54:26 -07:00
Josh Story f4f873f628 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...
2023-04-10 14:51:54 -07:00
Sebastian Markbåge 9a9da7721e 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.
2023-04-09 22:16:38 -04:00
Sebastian Markbåge e5146cb525 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.
2023-04-09 18:06:16 -04:00
Sebastian Markbåge 9cfba0f6ec 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.
2023-04-05 19:38:20 -04:00
Sophie Alpert 790ebc962d Remove no-fallthrough lint suppressions (#26553)
The lint rule already understands a normal comment. Also a bunch of
these were unnecessary.
2023-04-04 20:08:33 -07:00
Sebastian MarkbågeandSophie Alpert c15579631f Put common aliases in Map/Set instead of switch over strings (#26551)
This is a follow up to https://github.com/facebook/react/pull/26546

This is strictly a perf optimization since we know that switches over
strings aren't optimally implemented in current engines. Basically
they're a sequence of ifs.

As a result, we're better off putting the unusual cases in a Map and the
very common cases in the beginning of the switch. We might be better off
putting very common cases in explicit ifs - just in case the engine does
optimize switches to a hash table which is potentially worse.

---------

Co-authored-by: Sophie Alpert <git@sophiebits.com>
2023-04-04 18:06:09 -04:00
Mohammad Ghorbani d5fd60f7e6 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.
2023-04-04 12:37:14 -04:00
Sebastian Markbåge eeabb7312f 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.
2023-04-04 11:05:56 -04:00
Josh Story 4a1cc2ddd0 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
2023-04-03 09:34:53 -07:00
Andrew Clark 0ae348018d [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.
2023-03-31 15:45:45 -04:00
Andrew Clark 8310854ceb 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.
2023-03-31 10:25:58 -04:00
Sebastian Markbåge 43a70a610d 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
2023-03-30 18:38:50 -04:00
Sebastian Markbåge 1a1d61fed9 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
2023-03-30 16:31:15 -04:00
Sebastian Markbåge 73deff0d51 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.
2023-03-30 14:30:57 -04:00
Sebastian Markbåge 29a3be78bd Move ReactDOMFloat to react-dom/src/ (#26514)
This is not really part of the bindings, it's more part of the package
entry points. /shared/ is not really right neither because it's more
like an isomorphic entry point and not some utility.
2023-03-29 23:39:24 -04:00
Sebastian Markbåge 4c2fc01900 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.
2023-03-29 23:39:02 -04:00
Sebastian Markbåge 85de6fde51 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.
2023-03-28 22:40:03 -04:00
Jan Kassens afea1d0c53 [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
2023-03-27 13:43:04 +02:00
Andrew Clark 768f965de2 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.
2023-03-26 23:48:37 -04:00
Josh Story 73b6435ca4 [Float][Fiber] Implement waitForCommitToBeReady for stylesheet resources (#26450)
Before a commit is finished if any new stylesheet resources are going to
mount and we are capable of delaying the commit we will do the following

1. Wait for all preloads for newly created stylesheet resources to load
2. Once all preloads are finished we insert the stylesheet instances for
these resources and wait for them all to load
3. Once all stylesheets have loaded we complete the commit

In this PR I also removed the synchronous loadingstate tracking in the
fizz runtime. It was not necessary to support the implementation on not
used by the fizz runtime itself. It makes the inline script slightly
smaller

In this PR I also integrated ReactDOMFloatClient with
ReactDOMHostConfig. It leads to better code factoring, something I
already did on the server a while back. To make the diff a little easier
to follow i make these changes in a single commit so you can look at the
change after that commit if helpful

There is a 500ms timeout which will finish the commit even if all
suspended host instances have not finished loading yet

At the moment error and load events are treated the same and we're
really tracking whether the host instance is finished attempting to
load.
2023-03-24 19:17:38 -07:00
Sebastian MarkbågeandRick Hanlon afb3d51dc6 Fix enableClientRenderFallbackOnTextMismatch flag (#26457)
With this flag off, we don't throw and therefore don't patch up the tree
when suppression is off.

Haven't tested.

---------

Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>
2023-03-22 13:12:41 -04:00
Sebastian Markbåge b93b4f0745 Should not throw for children of iframe or object (#26458)
Still needs a regression test to test this for the future.
2023-03-22 13:56:20 +01:00
Sebastian Markbåge 520f7f3ed4 Refactor ReactDOMComponent to use flatter property operations (#26433)
This is in line with the refactor I already did on Fizz earlier and
brings Fiber up to a similar structure.

We end up with a lot of extra checks due the extra abstractions we use
to check the various properties. This uses a flatter and more inline
model which makes it easier to see what each property does. The tradeoff
is that a change might need changes in more places.

The general structure is that there's a switch for tag first, then a
switch for each attribute special case, then a switch for the value. So
it's easy to follow where each scenario will end up and there shouldn't
be any unnecessary code executed along the way.

My goal is to eventually get rid of the meta-programming in DOMProperty
and CSSProperty but I'm leaving that in for now - in line with Fizz.

My next step is moving around things a bit in the diff/commit phases.
This is the first step to more refactors for perf and size, but also
because I'm adding more special cases so I need to have a flatter
structure that I can reason about for those special cases.
2023-03-20 20:56:14 -04:00
Andrew Clark 0131d0cff4 Check if suspensey instance resolves in immediate task (#26427)
When rendering a suspensey resource that we haven't seen before, it may
have loaded in the background while we were rendering. We should yield
to the main thread to see if the load event fires in an immediate task.

For example, if the resource for a link element has already loaded, its
load event will fire in a task right after React yields to the main
thread. Because the continuation task is not scheduled until right
before React yields, the load event will ping React before it resumes.

If this happens, we can resume rendering without showing a fallback.

I don't think this matters much for images, because the `completed`
property tells us whether the image has loaded, and during a non-urgent
render, we never block the main thread for more than 5ms at a time (for
now — we might increase this in the future). It matters more for
stylesheets because the only way to check if it has loaded is by
listening for the load event.

This is essentially the same trick that `use` does for userspace
promises, but a bit simpler because we don't need to replay the host
component's begin phase; the work-in-progress fiber already completed,
so we can just continue onto the next sibling without any additional
work.

As part of this change, I split the `shouldSuspendCommit` host config
method into separate `maySuspendCommit` and `preloadInstance` methods.
Previously `shouldSuspendCommit` was used for both.

This raised a question of whether we should preload resources during a
synchronous render. My initial instinct was that we shouldn't, because
we're going to synchronously block the main thread until the resource is
inserted into the DOM, anyway. But I wonder if the browser is able to
initiate the preload even while the main thread is blocked. It's
probably a micro-optimization either way because most resources will be
loaded during transitions, not urgent renders.
2023-03-20 12:35:10 -04:00
Andrew Clark db281b3d9c Feature: Suspend commit without blocking render (#26398)
This adds a new capability for renderers (React DOM, React Native):
prevent a tree from being displayed until it is ready, showing a
fallback if necessary, but without blocking the React components from
being evaluated in the meantime.

A concrete example is CSS loading: React DOM can block a commit from
being applied until the stylesheet has loaded. This allows us to load
the CSS asynchronously, while also preventing a flash of unstyled
content. Images and fonts are some of the other use cases.

You can think of this as "Suspense for the commit phase". Traditional
Suspense, i.e. with `use`, blocking during the render phase: React
cannot proceed with rendering until the data is available. But in the
case of things like stylesheets, you don't need the CSS in order to
evaluate the component. It just needs to be loaded before the tree is
committed. Because React buffers its side effects and mutations, it can
do work in parallel while the stylesheets load in the background.

Like regular Suspense, a "suspensey" stylesheet or image will trigger
the nearest Suspense fallback if it hasn't loaded yet. For now, though,
we only do this for non-urgent updates, like with startTransition. If
you render a suspensey resource during an urgent update, it will revert
to today's behavior. (We may or may not add a way to suspend the commit
during an urgent update in the future.)

In this PR, I have implemented this capability in the reconciler via new
methods added to the host config. I've used our internal React "no-op"
renderer to write tests that demonstrate the feature. I have not yet
implemented Suspensey CSS, images, etc in React DOM. @gnoff and I will
work on that in subsequent PRs.
2023-03-17 18:05:11 -04:00
Sebastian Markbåge 8fa41ffa27 Don't "fix up" mismatched text content with suppressedHydrationWarning (#26391)
In concurrent mode we error if child nodes mismatches which triggers a
recreation of the whole hydration boundary. This ensures that we don't
replay the wrong thing, transform state or other security issues.

For text content, we respect `suppressedHydrationWarning` to allow for
things like `<div suppressedHydrationWarning>{timestamp}</div>` to
ignore the timestamp. This mode actually still patches up the text
content to be the client rendered content.

In principle we shouldn't have to do that because either value should be
ok, and arguably it's better not to trigger layout thrash after the
fact.

We do have a lot of code still to deal with patching up the tree because
that's what legacy mode does which is still in the code base. When we
delete legacy mode we would still be stuck with a lot of it just to deal
with this case.

Therefore I propose that we change the semantics to not patch up
hydration errors for text nodes. We already don't for attributes.
2023-03-16 22:39:17 -04:00
Sebastian Markbåge cfc1274e3b Disable IE innerHTML workaround behind a flag (#26390)
We don't need this workaround for SVG anymore and we don't need to
workaround MSApp's security model since Windows 10.
2023-03-14 23:27:04 -04:00
Sebastian Markbåge a57f40d839 Undo dependency injection of batching (#26389)
There's currently a giant cycle between the event system, through
react-dom-bindings, reconciler and then react-dom. We resolve this cycle
using dependency injection. However, this all ends up in the same
bundle. It can be reordered to resolve the cycles. If we avoid
side-effects and avoid reading from module exports during
initialization, this should be resolvable in a more optimal way by the
compiler.
2023-03-14 21:40:43 -04:00
Sebastian Markbåge d310d654a7 Avoid meta programming to initialize functions in module scope (#26388)
I'm trying to get rid of all meta programming in the module scope so
that closure can do a better job figuring out cyclic dependencies and
ability to reorder.

This is converting a lot of the patterns that assign functions
conditionally to using function declarations instead.

```
let fn;
if (__DEV__) {
  fn = function() {
    ...
  };
}
```
->
```
function fn() {
  if (__DEV__) {
    ...
  }
}
```
2023-03-14 21:00:22 -04:00
Sebastian Markbåge 21aee59e45 Delete unused DOM files (#26387)
These used to be used by partial render.

ReactDOMDispatcher ended up not being used in this way.

Move shared DOM files to client. These are only used by client
abstractions now. They're inlined in the Fizz code so they're no longer
shared.
2023-03-14 19:52:57 -04:00
Sebastian Markbåge 2788d0d8dd Allow empty string to be passed to formAction (#26379)
We disallow empty strings for `href` and `src` since they're common
mistakes that end up loading the current page as a preload, image or
link. We also disallow it for `action`. You have to pass `null` which is
the same.

However, for `formAction` passing `null` is not the same as passing
empty string. Passing empty string overrides the form's action to be the
current page even if the form's action was set to something else.
There's no easy way to express the same thing `#` show up in the user
visible URLs and `?` clears the search params.

Since this is also not a common mistake, we can just allow this.
2023-03-13 14:28:17 -04:00
Josh Story 3706edb81c [Float][Fizz]: Don't preload nomodule scripts (#26353)
We attempt to preload scripts that we detect during Fizz rendering
however when `noModule={true}` we should not because it will force
modern browser to fetch scripts they will never execute

Hoisted script resources already don't preload because we just emit the
resource immediately. This change currently on affects the preloads for
scripts that aren't hoistable
2023-03-08 21:59:43 -08:00
Josh Story 978fae4b4f [Float][Fiber] implement a faster hydration match for hoistable elements (#26154)
This PR is now based on #26256 

The original matching function for `hydrateHoistable` some challenging
time complexity since we built up the list of matchable nodes for each
link of that type and then had to check to exclusion. This new
implementation aims to improve the complexity

For hoisted title tags we match the first title if it is valid (not in
SVG context and does not have `itemprop`, the two ways you opt out of
hoisting when rendering titles). This path is much faster than others
and we use it because valid Documents only have 1 title anyway and if we
did have a mismatch the rendered title still ends up as the
Document.title so there is no functional degradation for misses.

For hoisted link and meta tags we track all potentially hydratable
Elements of this type in a cache per Document. The cache is refreshed
once each commit if and only if there is a title or meta hoistable
hydrating. The caches are partitioned by a natural key for each type
(href for link and content for meta). Then secondary attributes are
checked to see if the potential match is matchable.

For link we check `rel`, `title`, and `crossorigin`. These should
provide enough entropy that we never have collisions except is contrived
cases and even then it should not affect functionality of the page. This
should also be tolerant of links being injected in arbitrary places in
the Document by 3rd party scripts and browser extensions

For meta we check `name`, `property`, `http-equiv`, and `charset`. These
should provide enough entropy that we don't have meaningful collisions.
It is concievable with og tags that there may be true duplciates `<meta
property="og:image:size:height" content="100" />` but even if we did
bind to the wrong instance meta tags are typically only read from SSR by
bots and rarely inserted by 3rd parties so an adverse functional outcome
is not expected.
2023-03-06 19:52:35 -08:00
Josh Story 8a9f82ed58 [Float][Fizz][Fiber] - Do not hoist elements with itemProp & hydrate more tolerantly in hoist contexts (#26256)
## Do not hoist elements with `itemProp`
In HTML `itemprop` signifies a property of an `itemscope` with respect
to the Microdata spec
(https://html.spec.whatwg.org/multipage/microdata.html#microdata)

additionally `itemprop` is valid on any tag and can even make some tags
that are otherwise invalid in the `<body>` valid there (`<meta>` for
instance).

Originally I tried an approach where if you rendered something otherwise
hoistable inside an `itemscope` it would not hoist if it had an
`itemprop`. This meant that some components with `itemprop` could hoist
(if they were not scoped, which is generally invalid microdata
implementation). However the problem is things that do hoist, hoist into
the head and body and these tags can have an `itemscope`. This creates a
ton of ambiguity when trying to hydrate in these hoist scopes because we
can't know for certain whether a DOM node we find there was hoisted or
not even if it has an `itemprop` attribute. There are other scenarios
too that have abiguous semantics like rendering a hoistable with
`itemProp` outside of `<html itemScope={true>`. Is it fair to embed that
hoistable inside that itemScope even though it was defined outside?

To simplify the situation and disambiguate I dropped the `itemscope`
portion from the implementation and now any host component that could
normally be hoisted will not hoist if it has an `itemProp` prop.

In addition to the changes made for `itemProp` this PR also modifies
part of the hydration implementation to be more tolerant of tags
injected by 3rd parties. This was opportunistically done when we needed
to have context information like `inItemScope` but with the most recent
implementation that has been removed. I have however left the hydration
changes in place as it is a goal to make React handle hydrating the
entire Document even when we cannot control whether 3rd parties are
going to inject tags that React will not render but are also not
hoistables

-------

##### Original Description when we considered tracking itemScope
>One recent decision was to make elements using the `itemProp` prop not
hoistable if they were inside and itemScope. This better fits with
Microdata spec which allows for meta tags and other tag types usually
reserved for the `<head>` to be used in the `<body>` when using
itemScope.
>
>To implement this a number of small changes were necessary
>
>1. HostContext in prod needed to expand beyond just tracking the
element namespace for new element creation. It now tracks whether we are
in an itemScope. To keep this efficient it is modeled as a bitmask.
>2. To disambiguate what is and is not a potential instance in the DOM
for hoistables the hydration algo was updated to skip past non-matching
instances while attempting to claim the instance rather than ahead of
time (getNextHydratable).
>3. React will not consider an itemScope on `<html>`, `<head>`, or
`<body>` as a valid scope for the hoisting opt-out. This is important as
an invariant so we can make assumptions about certain tags in these
scopes. This should not be a functional breaking change because if any
of these tags have an `itemScope` then it can just be moved into the
first node inside the `<body>`
>
>Since we were already updating the logic for hydration to better
support `itemScope` opt-out I also changed the hydration behavior for
suspected 3rd party nodes in `<head>` and `<body>`. Now if you are
hydrating in either of those contexts hydration will skip past any
non-matching nodes until it finds a match. This allows 3rd party scripts
and extensions to inject nodes in either context that React does not
expect and still avoid a hydration mismatch.
>
>This new algorithm isn't perfect and it is possible for a mismatch to
occur. The most glaring case may be if a 3rd party script prepends a
`<div>` into `<body>` and you render a `<div>` in `<body>` in your app.
there is nothing to signal to React that this div was 3rd party so it
will claim is as the hydrated instance and hydration will almost
certainly fail immediately afterwards.
>
>The expectation is that this is rare and that if falling back to client
rendering is transparent to the user then there is not problem here. We
will continue to evaluate this and may change the hydration matching
algorithm further to match user and developer expectations
2023-03-06 15:00:54 -08:00