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.
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.
This deletes the ReactIncrementalTriangle test suite, which I originally
added back in 2017 when I was working on Fiber's "resuming" feature. It
was meant to simulate a similar scenario as Seb's "Sierpinski Triangle"
Fiber demo.
We eventually ended up removing resuming, but we kept this fuzz tester
around since it wasn't really harming anything. However, over the years,
we've had to make many small tweaks to decouple it from implementation
details, to the point that it doesn't test anything useful anymore. And
the thing that it originally tested has long since been removed.
If or when we do add back resuming, we would write a different fuzz
tester from scratch rather than build on this one.
So rather than continue to contrive ways to prevent it from breaking, I
propose we delete it.
We still have other fuzz testers for things like Suspense and context
propagation. Only this particular one has outlived its usefulness.
## Summary
With https://github.com/facebook/react/pull/26349 we now serialize
`undefined`. However, deserializing it on the client is currently
indistinguishable from the value missing entirely due to how
`JSON.parse` treats `undefined` return value of reviver functions.
This leads to inconsistent behavior of the `Object.hasOwn` or `in`
operator (used for narrowing in TypeScript). In TypeScript-speak, `{
prop: T | undefined}` will arrive as `{ prop?: T }`.
## How did you test this change?
- Added test that is expected to fail. Though ideally the implementation
of the component would not care whether it's used on the client or
server.
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.
This updates the Suspense fuzz tester to use `act` to recursively flush
timers instead of doing it manually.
This still isn't great because ideally the fuzz tester wouldn't fake
timers at all. It should resolve promises using a custom queue instead
of Jest's fake timer queue, like we've started doing in our other
Suspense tests (i.e. the `resolveText` pattern). That's because our
internal `act` API (not the public one, the one we use in our tests)
uses Jest's fake timer queue as a way to force Suspense fallbacks to
appear.
However I'm not interested in upgrading this test suite to a better
strategy right now because if I were writing a Suspense fuzzer today I
would probably use an entirely different approach. So this is just an
incremental improvement to make it slightly less decoupled to React
implementation details.
I rewrote some of our tests that deal with microtasks with the aim of
making them less coupled to implementation details. This is related to
an upcoming change to move update processing into a microtask.
Continuing my journey to migrate all the Scheduler flush* methods to
async versions of the same helpers.
`unstable_flushExpired` is a rarely used helper that is only meant to be
used to test a very particular implementation detail (update starvation
prevention, or what we sometimes refer to as "expiration").
I've prefixed the new helper with `unstable_`, too, to indicate that our
tests should almost always prefer one of the other patterns instead.
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
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.
## Summary
Just copied the types over from the internal types. Type error was
hidden by overly broad FlowFixMe. With `$FlowFixMe[not-a-function]` we
would've seen the actual issue:
```
Cannot return `dispatcher.useEffectEvent(...)` because `T` [1] is incompatible with undefined [2].Flow(incompatible-return)
```
## How did you test this change?
- [x] yarn flow dom-node
- [x] CI
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.
Follow-up to https://github.com/facebook/react/pull/26442.
It looks like we missed a few cases where we default import a CommonJS
module, which leads to Rollup adding `.default` access, e.g.
`require('webpack/lib/Template').default` in the output.
To fix, add the remaining cases to the list of exceptions. Verified by
going through all `externals` in the bundle list, and manually checking
the webpack plugin.
<!--
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 test --debug --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
This PR:
- Updates Rollup from 2.x to latest 3.x, and updates associated plugins
- Updates deprecated / altered config settings in the Rollup plugin
pipeline
- Fixes some file extension and import issues related to use of ESM in
`react-dom-webpack-server`
- Removes a now-obsolete `strip-unused-imports` Rollup plugin
- <s>Fixes an _existing_ bug with the Rollup 2.x plugin pipeline on
`main` that was causing parts of `DOMProperty.js` to get left out of the
`react-dom-webpack-server` JS bundles, by adding a new plugin to tell
Rollup to treat that file as if it as side effects</s>
This PR should be functionally identical to the other existing "Rollup 3
upgrade" PR at #26078 . I'm filing this as a near-duplicate because I'm
ready to push this change through ASAP so that I can follow it up with a
PR that adds sourcemap support, that PR's artifact diffing seems like
it's possibly stuck and I want to compare the build results, and I've
got this set up against latest `main`.
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
This gets React's build setup updated to the latest Rollup version,
which is generally a good practice, but also ensures that any further
Rollup config tweaks can be done using the current Rollup docs as a
reference.
## How did you test this change?
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
- Made builds from the latest `main`
- Updated Rollup package versions and cross-compared the changes I
needed to make locally to get successful builds vs #26078
- Diffed the output folders between `main` and this PR, and confirmed
that the bundle contents are identical (with the exception of version
strings and the `react-dom-webpack-server` bundle fix re-adding missing
`DOMProperty.js` content)
## Summary
Now that React Native owns the definition for public instances in Fabric
and ReactNativePrivateInterface provides the methods to create instances
and access private fields (see
https://github.com/facebook/react-native/pull/36570), we can remove the
definitions from React.
After this PR, React Native public instances will be opaque types for
React and it will only handle their creation but not their definition.
This will make RN similar to DOM in how public instances are handled.
This is a new version of #26418 which was closed without merging.
## How did you test this change?
* Existing tests.
* Manually synced the changes in this PR to React Native and tested it
end to end in Meta's infra.
## Overview
This PR unfortunately removes the warning emitted when using layout
effects on the server:
> useLayoutEffect does nothing on the server, because its effect cannot
be encoded into the server renderer's output format. This will lead to a
mismatch between the initial, non-hydrated UI and the intended UI. To
avoid this, useLayoutEffect should only be used in components that
render exclusively on the client. See
https://reactjs.org/link/uselayouteffect-ssr for common fixes.
## Why this warning exists
The new docs explain this really well. Adding a screenshot because as
part of this change, we'll be removing these docs.
<img width="1562" alt="Screenshot 2023-03-15 at 10 56 17 AM"
src="https://user-images.githubusercontent.com/2440089/225349148-f0e57c3f-95f5-4f2e-9178-d9b9b221c28d.png">
## Why are we changing it
In practice, users are not just ignoring this warning, but creating
hooks to bypass this warning by switching the useLayoutEffect hook on
the server instead of fixing it. This battle seems to be lost, so let's
remove the warning so at least users don't need to use the indirection
hook any more. In practice, if it's an issue, you should see the
problems like flashing the wrong content on first load in development.
Fixes a bug in SuspenseList that @kassens found when deploying React to
Meta. In some scenarios, SuspenseList would force the fallback of a
deeply nested Suspense boundary into fallback mode, which should never
happen under any circumstances — SuspenseList should only affect the
nearest descendent Suspense boundaries, without going deeper.
The cause was that the internal ForceSuspenseFallback context flag was
not being properly reset when it reached the nearest Suspense boundary.
It should only be propagated shallowly.
We didn't discover this earlier because the scenario where it happens is
not that common. To trigger the bug, you need to insert a new Suspense
boundary into an already-mounted row of the list. But often when a new
Suspense boundary is rendered, it suspends and shows a fallback, anyway,
because its content hasn't loaded yet.
Another reason we didn't discover this earlier is because there was
another bug that was accidentally masking it, which was fixed by #25922.
When that fix landed, it revealed this bug.
The SuspenseList implementation is complicated but I'm not too concerned
with the current messiness. It's an experimental API, and we intend to
release it soon, but there are some known flaws and missing features
that we need to address first regardless. We'll likely end up rewriting
most of it.
Co-authored-by: Jan Kassens <jkassens@meta.com>
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>
## Summary
This type was defined as `mixed` to avoid bringing the whole definition
from React to React Native, but its definition is visible to RN. This
type should be opaque to RN, so this makes it explicit.
## How did you test this change?
Applied the same changes in the React Native repository and could use
the type without issues.
## Summary
Update readme to new documentation links to
[`React.useSyncExternalStore`](https://react.dev/reference/react/useSyncExternalStore
## How did you test this change?
This is just a documentation change, so we don't need to test it
Co-authored-by: 田源 <tianyuan@eol.cn>
Today if something suspends, React will continue rendering the siblings
of that component.
Our original rationale for prerendering the siblings of a suspended
component was to initiate any lazy fetches that they might contain. This
was when we were more bullish about lazy fetching being a good idea some
of the time (when combined with prefetching), as opposed to our latest
thinking, which is that it's almost always a bad idea.
Another rationale for the original behavior was that the render was I/O
bound, anyway, so we might as do some extra work in the meantime. But
this was before we had the concept of instant loading states: when
navigating to a new screen, it's better to show a loading state as soon
as you can (often a skeleton UI), rather than delay the transition.
(There are still cases where we block the render, when a suitable
loading state is not available; it's just not _all_ cases where
something suspends.) So the biggest issue with our existing
implementation is that the prerendering of the siblings happens within
the same render pass as the one that suspended — _before_ the loading
state appears.
What we should do instead is immediately unwind the stack as soon as
something suspends, to unblock the loading state.
If we want to preserve the ability to prerender the siblings, what we
could do is schedule special render pass immediately after the fallback
is displayed. This is likely what we'll do in the future. However, in
the new implementation of `use`, there's another reason we don't
prerender siblings: so we can preserve the state of the stack when
something suspends, and resume where we left of when the promise
resolves without replaying the parents. The only way to do this
currently is to suspend the entire work loop. Fiber does not currently
support rendering multiple siblings in "parallel". Once you move onto
the next sibling, the stack of the previous sibling is discarded and
cannot be restored. We do plan to implement this feature, but it will
require a not-insignificant refactor.
Given that lazy data fetching is already bad for performance, the best
trade off for now seems to be to disable prerendering of siblings. This
gives us the best performance characteristics when you're following best
practices (i.e. hoist data fetches to Server Components or route
loaders), at the expense of making an already bad pattern a bit worse.
Later, when we implement resumable context stacks, we can reenable
sibling prerendering. Though even then the use case will mostly be to
prerender the CPU-bound work, not lazy fetches.
(This was reviewed and approved as part of #26380; I'm extracting it
into its own PR so that it can bisected later if it causes an issue.)
I noticed while working on a PR that when an error happens during
hydration, and we revert to client rendering, React actually does _two_
additional render passes instead of just one. We didn't notice it
earlier because none of our tests happened to assert on how many renders
it took to recover, only on the final output.
It's possible this extra render pass had other consequences that I'm not
aware of, like messing with some assumption in the recoverable errors
logic.
This adds a test to demonstrate the issue. (One problem is that we don't
have much test coverage of this scenario in the first place, which
likely would have caught this earlier.)
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.
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.
## Summary
We are going to move the definition of public instances from React to
React Native to have them together with the native methods in Fabric
that they invoke. This will allow us to have a better type safety
between them and iterate faster on the implementation of this proposal:
https://github.com/react-native-community/discussions-and-proposals/pull/607
The interface between React and React Native would look like this after
this change and a following PR (#26418):
React → React Native:
```javascript
ReactNativePrivateInterface.createPublicInstance // to provide via refs
ReactNativePrivateInterface.getNodeFromPublicInstance // for DevTools, commands, etc.
ReactNativePrivateInterface.getNativeTagFromPublicInstance // to implement `findNodeHandle`
```
React Native → React (ReactFabric):
```javascript
ReactFabric.getNodeFromInternalInstanceHandle // to get most recent node to call into native
ReactFabric.getPublicInstanceFromInternalInstanceHandle // to get public instances from results from native
```
## How did you test this change?
Flow
Existing unit tests
Sizebot works by fetching the base artifacts from CI. CircleCI recently
updated this endpoint to require an auth token. This is a problem for PR
branches, where sizebot runs, because we don't want to leak the token to
arbitrary code written by an outside contributor.
This only affects PR branches. CI workflows that run on the main branch
are allowed to access environment variables, because only those with
push access can land code in main.
As a temporary workaround, we'll fetch the assets from a mirror,
react-builds.vercel.app. This is the same app that hosts the sizebot
diff previews.
Need to figure out a longer term solution. Perhaps by converting sizebot
into a proper GitHub app.
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.
CircleCI now enforces passing a token when fetching artifacts. I'm also
deleting the old request-promise-json dependency because AFAIK we were
only using it to fetch json from circleci about the list of available
artifacts – which we can just do using node-fetch. Plus, the underlying
request package it uses has been deprecated since 2019.
## Summary
We had to revert the last React sync to React Native because we saw
issues with Responder events using stale event handlers instead of
recent versions.
I reviewed the merged PRs and realized the problem was in the refactor I
did in #26321. In that PR, we moved `currentProps` from `canonical`,
which is a singleton referenced by all versions of the same fiber, to
the fiber itself. This is causing the staleness we observed in events.
This PR does a partial revert of the refactor in #26321, bringing back
the `canonical` object but moving `publicInstance` to one of its fields,
instead of being the `canonical` object itself.
## How did you test this change?
Existing unit tests continue working (I didn't manage to get a repro
using the test renderer).
I manually tested this change in Meta infra and saw the problem was
fixed.
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.
Sebastian Markbåge
