Fixes: #27200
preloads for images that appear before the viewport meta may be loaded
twice because the proper device image information is not used with the
preload but is with the image itself. The viewport meta should be
emitted earlier than all preloads to avoid this.
this change moves the queue for the viewport meta to preconnects which
already has the right priority for this tag
<!--
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
- Remove unused webpack 4 dependencies
## How did you test this change?
- Ran `yarn test --prod`
- Ran `yarn test`
## Related PRs:
- https://github.com/facebook/react/pull/26887
## Summary
Since we are enabling `useModernStrictMode` flag internally, to make
sure the internal testing of half StrictMode doesn't suddenly break,
this PR makes sure it also works with `useModernStrictMode` true.
## Test plan:
Manually set `useModernStrictMode` to true.
`yarn test ReactOffscreenStrictMode-test -r=www-modern --env=development
--variant=true`
`yarn test ReactStrictMode-test.internal -r=www-modern --env=development
--variant=true`
## Summary
This was not exposed as a dynamic flag in the build for facebook www. By
adding it, we'll be able to roll this out incrementally before cleaning
up this code altogether.
## How did you test this change?
`yarn build`
Before changes, `disableSchedulerTimeoutInWorkLoop` flag is not included
in ReactDOM-* build output for facebook www. Afterwards, it is included.
## Summary
`scheduler.yield` is entering [Origin Trial soon in Chrome
115](https://chromestatus.com/feature/6266249336586240). This diff adds
it to `SchedulerPostTask` when scheduling continuations to allow Origin
Trial participation for early feedback on the new API.
It seems the difference here versus the current use of `postTask` will
be minor – the intent behind `scheduler.yield` seems to mostly be better
ergonomics for scheduling continuations, but it may be interesting to
see if the follow aspect of it results in any tangible difference in
scheduling (from
[here](https://github.com/WICG/scheduling-apis/blob/main/explainers/yield-and-continuation.md#introduction)):
> To mitigate yielding performance penalty concerns, UAs prioritize
scheduler.yield() continuations over tasks of the same priority or
similar task sources.
## How did you test this change?
```
yarn test SchedulerPostTask
```
We already did this for Server References on the Client so this brings
us parity with that. This gives us some more flexibility with changing
the runtime implementation without having to affect the loaders.
We can also do more in the runtime such as adding `.bind()` support to
Server References.
I also moved the CommonJS Proxy creation into the runtime helper from
the register so that it can be handled in one place.
This lets us remove the forks from Next.js since the loaders can be
simplified there to just use these helpers.
This PR doesn't change the protocol or shape of the objects. They're
still specific to each bundler but ideally we should probably move this
to shared helpers that can be used by multiple bundler implementations.
## Summary
as we began [discussing
yesterday](https://github.com/facebook/react/pull/27056#discussion_r1253282784),
`SuspenseList` is not actually stable yet, and should likely be exported
with the `unstable_` prefix.
the conversation yesterday began discussing this in the context of the
fb-specific packages, but changing it there without updating everywhere
else leads to test failures, so here the change is made across packages.
## How did you test this change?
```
yarn flow dom-browser
yarn test
```
When selecting a package variant from an export map we should favor node
over edge-light
edge-light represents a runtime with some minimal set of web apis
generally found across edge runtimes. However some environments might be
both edge-light compatible and node compatible and (node is adding many
web APIs) and when both conditions exist we want to favor the node
implementations. A followup to this change will add the web streams APIs
to Flight and Fizz so the node version exports the same interfaces for
web streams that edge does in addition to the node specific
implementations.
## Summary
came across these TODOs – an internal grep indicated that remaining
callsites have been cleaned up, so these can now be removed.
## How did you test this change?
```
yarn flow dom-browser
yarn test
```
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.
React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.
I added this to the existing "Rules of Hooks" ESLint rule.
Adds a development warning to complement the error introduced by
https://github.com/facebook/react/pull/27019.
We can detect and warn about async client components by checking the
prototype of the function. This won't work for environments where async
functions are transpiled, but for native async functions, it allows us
to log an earlier warning during development, including in cases that
don't trigger the infinite loop guard added in
https://github.com/facebook/react/pull/27019. It does not supersede the
infinite loop guard, though, because that mechanism also prevents the
app from crashing.
I also added a warning for calling a hook inside an async function. This
one fires even during a transition. We could add a corresponding warning
to Flight, since hooks are not allowed in async Server Components,
either. (Though in both environments, this is better handled by a lint
rule.)
Modern runtimes support native async/await, as does the version of Node
we use for our tests. To match how most of our users run React, this
disables the transpilation of async/await in our test suite.
`unstable_batchedUpdates` shouldn't really be called on the server but
the semantics are clear enough and we can provide a trivial
implementation that calls the provided callback so we are adding it to
the server-rendering-stub.
This means we will also keep it around when we make the top level
react-dom exports match the server-rendering-stub in the next major
Suspending with an uncached promise is not yet supported. We only
support suspending on promises that are cached between render attempts.
(We do plan to partially support this in the future, at least in certain
constrained cases, like during a route transition.)
This includes the case where a component returns an uncached promise,
which is effectively what happens if a Client Component is authored
using async/await syntax.
This is an easy mistake to make in a Server Components app, because
async/await _is_ available in Server Components.
In the current behavior, this can sometimes cause the app to crash with
an infinite loop, because React will repeatedly keep trying to render
the component, which will result in a fresh promise, which will result
in a new render attempt, and so on. We have some strategies we can use
to prevent this — during a concurrent render, we can suspend the work
loop until the promise resolves. If it's not a concurrent render, we can
show a Suspense fallback and try again at concurrent priority.
There's one case where neither of these strategies work, though: during
a sync render when there's no parent Suspense boundary. (We refer to
this as the "shell" of the app because it exists outside of any loading
UI.)
Since we don't have any great options for this scenario, we should at
least error gracefully instead of crashing the app.
So this commit adds a detection mechanism for render loops caused by
async client components. The way it works is, if an app suspends
repeatedly in the shell during a synchronous render, without committing
anything in between, we will count the number of attempts and eventually
trigger an error once the count exceeds a threshold.
In the future, we will consider ways to make this case a warning instead
of a hard error.
See https://github.com/facebook/react/issues/26801 for more details.
This uses the same mechanism as [large
strings](https://github.com/facebook/react/pull/26932) to encode chunks
of length based binary data in the RSC payload behind a flag.
I introduce a new BinaryChunk type that's specific to each stream and
ways to convert into it. That's because we sometimes need all chunks to
be Uint8Array for the output, even if the source is another array buffer
view, and sometimes we need to clone it before transferring.
Each type of typed array is its own row tag. This lets us ensure that
the instance is directly in the right format in the cached entry instead
of creating a wrapper at each reference. Ideally this is also how
Map/Set should work but those are lazy which complicates that approach a
bit.
We assume both server and client use little-endian for now. If we want
to support other modes, we'd convert it to/from little-endian so that
the transfer protocol is always little-endian. That way the common
clients can be the fastest possible.
So far this only implements Server to Client. Still need to implement
Client to Server for parity.
NOTE: This is the first time we make RSC effectively a binary format.
This is not compatible with existing SSR techniques which serialize the
stream as unicode in the HTML. To be compatible, those implementations
would have to use base64 or something like that. Which is what we'll do
when we move this technique to be built-in to Fizz.
Currently, only the browser build exposes the `$$FORM_ACTION` helper.
It's used for creating progressive enhancement fro Server Actions
imported from Client Components. This helper is only useful in SSR
builds so it should be included in the Edge/Node builds of the client.
I also removed it from the browser build. We assume that only the Edge
or Node builds of the client are used
together with SSR. On the client this feature is not needed so we can
exclude the code. This might be a bit unnecessary because it's not that
much code and in theory you might use SSR in a Service Worker or
something where the Browser build would be used but currently we assume
that build is only for the client. That's why it also don't take an
option for reverse
look up of file names.
Currently, since we use a module cache for async modules, it doesn't
automatically get updated when the module registry gets updated (HMR).
This technique ensures that if Webpack replaces the module (HMR) then
we'll get the new Promise when we require it again.
This technique doesn't work for ESM and probably not Vite since ESM will
provide a new Promise each time you call `import()` but in the
Webpack/CJS approach this Promise is an entry in the module cache and
not a promise for the entry.
I tried to replicate the original issue in the fixture but it's tricky
to replicate because 1) we can't really use async modules the same way
without compiling both server and client 2) even then I'm not quite sure
how to repro the HMR issue.
We already support these in the sense that they're Iterable so they just
get serialized as arrays. However, these are part of the Structured
Clone algorithm [and should be
supported](https://github.com/facebook/react/issues/25687).
The encoding is simply the same form as the Iterable, which is
conveniently the same as the constructor argument. The difference is
that now there's a separate reference to it.
It's a bit awkward because for multiple reference to the same value,
it'd be a new Map/Set instance for each reference. So to encode sharing,
it needs one level of indirection with its own ID. That's not really a
big deal for other types since they're inline anyway - but since this
needs to be outlined it creates possibly two ids where there only needs
to be one or zero.
One variant would be to encode this in the row type. Another variant
would be something like what we do for React Elements where they're
arrays but tagged with a symbol. For simplicity I stick with the simple
outlining for now.
This PR contains a regression test and two separate fixes: a targeted
fix, and a more general one that's designed as a last-resort guard
against these types of bugs (both bugs in app code and bugs in React).
I confirmed that each of these fixes separately are sufficient to fix
the regression test I added.
We can't realistically detect all infinite update loop scenarios because
they could be async; even a single microtask can foil our attempts to
detect a cycle. But this improves our strategy for detecting the most
common kind.
See commit messages for more details.
## Summary
This PR cleans up `useMutableSource`. This has been blocked by a
remaining dependency internally at Meta, but that has now been deleted.
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## How did you test this change?
```
yarn flow
yarn lint
yarn test --prod
```
<!--
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.
-->
Suppose that you have this setup for devtools test:
```
// @reactVersion <= 18.1
// @reactVersion >= 17.1
```
With previous implementation, the accumulated condition will be `"<=
18.1" && ">= 17.1"`, which is just `">= 17.1"`, when evaluated. That's
why we executed some tests for old versions of react on main (and
failed).
With these changes the resulting condition will be `"<= 18.1 >= 17.1"`,
not using `&&`, because semver does not support this operator. All
currently failing tests will be skipped now as expected.
Also increased timeout value for shell server to start
In https://github.com/facebook/react/pull/26914 I added an extra logic
to turn off double useEffect if there is an `Offscreen`
tag. But `Suspense` uses `Offscreen` tag internally and that turns off
`disableStrictPassiveEffect` for everything.
## Summary
Running `yarn test --project devtools --build` currently skips all
non-gated (without `@reactVersion` directives) devtools tests. This is
not expected behaviour, these changes are fixing it.
There were multiple related PRs to it:
- https://github.com/facebook/react/pull/26742
- https://github.com/facebook/react/pull/25712
- https://github.com/facebook/react/pull/24555
With these changes, the resulting behaviour will be:
- If `REACT_VERSION` env variable is specified:
- jest will not include all non-gated test cases in the test run
- jest will run only a specific test case, when specified
`REACT_VERSION` value satisfies the range defined by `@reactVersion`
directives for this test case
- If `REACT_VERSION` env variable is not specified, jest will run all
non-gated tests:
- jest will include all non-gated test cases in the test run
- jest will run all non-gated test cases, the only skipped test cases
will be those, which specified the range that does not include the next
stable version of react, which will be imported from `ReactVersions.js`
## How did you test this change?
Running `profilingCache` test suite without specifying `reactVersion`
now skips gated (>= 17 & < 18) test
<img width="1447" alt="Screenshot 2023-06-15 at 11 18 22"
src="https://github.com/facebook/react/assets/28902667/cad58994-2cb3-44b3-9eb2-1699c01a1eb3">
Running `profilingCache` test suite with specifying `reactVersion` to
`17` now runs this test case and skips others correctly
<img width="1447" alt="Screenshot 2023-06-15 at 11 20 11"
src="https://github.com/facebook/react/assets/28902667/d308960a-c172-4422-ba6f-9c0dbcd6f7d5">
Running `yarn test --project devtools ...` without specifying
`reactVersion` now runs all non-gated test cases
<img width="398" alt="Screenshot 2023-06-15 at 12 25 12"
src="https://github.com/facebook/react/assets/28902667/2b329634-0efd-4c4c-b460-889696bbc9e1">
Running `yarn test --project devtools ...` with specifying
`reactVersion` (to `17` in this example) now includes only gated tests
<img width="414" alt="Screenshot 2023-06-15 at 12 26 31"
src="https://github.com/facebook/react/assets/28902667/a702c27e-4c35-4b12-834c-e5bb06728997">
## Summary
Overlooked when was working on
https://github.com/facebook/react/pull/26887.
- Added `webpack` packages as dev dependencies to `react-devtools-core`,
because it calls webpack to build
- Added `process` package as dev dependency, because it is injected with
`ProvidePlugin` (otherwise fails with Safari usage)
- Updated rule for sourcemaps
- Listed required externals for `standalone` build
Tested on RN application & Safari
For React Native environment, we sometimes spam the console with
warnings `"Could not find Fiber with id ..."`.
This is an attempt to fix this or at least reduce the amount of such
potential warnings being thrown.
Now checking if fiber is already unnmounted before trying to get native
nodes for fiber. This might happen if you try to inspect an element in
DevTools, but at the time when event has been received, the element was
already unmounted.
<!--
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
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
We are upgrading React 17 codebase to React18, and `StrictMode` has been
great for surfacing potential production bugs on React18 for class
components. There are non-trivial number of test failures caused by
double `useEffect` in StrictMode. To prioritize surfacing and fixing
issues that will break in production now, we need a flag to turn off
double `useEffect` for now in StrictMode temporarily. This is a
Meta-only hack for rolling out `createRoot` and we will fast follow to
remove it and use full strict mode.
## 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.
-->
jest
When we diffInCommitPhase there's no updatePayload, which caused no
update to be applied.
This is unfortunate because it would've been a lot easier to see this
oversight if we didn't have to support both flags.
I also carified that updateHostComponent is unnecessary in the new flag.
We reuse updateHostComponent for HostSingleton and HostHoistables since
it has a somewhat complex path but that means you have to remember when
editing updateHostComponent that it's not just used for that tag.
Luckily with the new flag, this is actually unnecessary since we just
need to mark it for update if any props have changed and then we diff it
later.
For float methods the href argument is usually all we need to uniquely
key the request. However when preloading responsive images it is
possible that you may need more than one preload for differing
imagesizes attributes. When using imagesrcset for preloads the href
attribute acts more like a fallback href. For keying purposes the
imagesrcset becomes the primary key conceptually.
This change updates the keying logic for `ReactDOM.preload()` when you
pass `{as: "image"}`
1. If `options.imageSrcSet` is a non-emtpy string the key is defined as
`options.imageSrcSet + options.imageSizes`. The `href` argument is still
required but does not participate in keying.
2. If `options.imageSrcSet` is empty, missing, or an invalid format the
key is defined as the `href`. Changing the `options.imageSizes` does not
affect the key as this option is inert when not using
`options.imageSrcSet`
Additionally, currently there is a bug in webkit (Safari) that causes
preload links to fail to use imageSrcSet and fallback to href even when
the browser will correctly resolve srcset on an `<img>` tag. Because the
drawbacks of preloading the wrong image (href over imagesrcset) in a
modern browser outweight the drawbacks of not preloading anything for
responsive images in browsers that do not support srcset at all we will
omit the `href` attribute whenever `options.imageSrcSet` is provided. We
still require you provide an href since we want to be able to revert
this behavior once all major browsers support it
bug link: https://bugs.webkit.org/show_bug.cgi?id=231150
Some browsers, with some CSP configuration, will not preload a script if
the prelaod link tag does not provide a valid nonce attribute. This
change adds the ability to specify a nonce for `ReactDOM.preload(..., {
as: "script" })`
## Summary
- Updated `webpack` (and all related packages) to v5 in
`react-devtools-*` packages.
- I haven't touched any `TODO (Webpack 5)`. Tried to poke it, but each
my attempt failed and parsing hook names feature stopped working. I will
work on this in a separate PR.
- This work is one of prerequisites for updating Firefox extension to
manifests v3
related PRs:
https://github.com/facebook/react/pull/22267https://github.com/facebook/react/pull/26506
## How did you test this change?
Tested on all surfaces, explicitly checked that parsing hook names
feature still works.
## Summary
>Warning: Received NaN for the `children` attribute. If this is
expected, cast the value to a string.
Fixes this warning, when we try to display NaN as NaN in key-value list.
Follow up to #26932
For regular rows, we're increasing the index by one to skip past the
last trailing newline character which acts a boundary. For length
encoded rows we shouldn't skip an extra byte because it'll leave us
missing one.
This only accidentally worked because this was also the end of the
current chunk which tests don't account for since we're just passing
through the chunks. So I added some noise by splitting and joining the
chunks so that this gets tested.
Float types are currently spread out. this moves them to a single place
to ensure we properly handle the public type interface in all three
renderers.
This is a step towards moving the public interface and validation to a
common file shared by all three runtimes. Will also probably change the
function interface to be flatter
<!--
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
Fix devtools cannot be shutdown by bridge.shutdown().
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## 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.
-->
This introduces a Text row (T) which is essentially a string blob and
refactors the parsing to now happen at the binary level.
```
RowID + ":" + "T" + ByteLengthInHex + "," + Text
```
Today, we encode all row data in JSON, which conveniently never has
newline characters and so we use newline as the line terminator. We
can't do that if we pass arbitrary unicode without escaping it. Instead,
we pass the byte length (in hexadecimal) in the leading header for this
row tag followed by a comma.
We could be clever and use fixed or variable-length binary integers for
the row id and length but it's not worth the more difficult
debuggability so we keep these human readable in text.
Before this PR, we used to decode the binary stream into UTF-8 strings
before parsing them. This is inefficient because sometimes the slices
end up having to be copied so it's better to decode it directly into the
format. The follow up to this is also to add support for binary data and
then we can't assume the entire payload is UTF-8 anyway. So this
refactors the parser to parse the rows in binary and then decode the
result into UTF-8. It does add some overhead to decoding on a per row
basis though.
Since we do this, we need to encode the byte length that we want decode
- not the string length. Therefore, this requires clients to receive
binary data and why I had to delete the string option.
It also means that I had to add a way to get the byteLength from a chunk
since they're not always binary. For Web streams it's easy since they're
always typed arrays. For Node streams it's trickier so we use the
byteLength helper which may not be very efficient. Might be worth
eagerly encoding them to UTF8 - perhaps only for this case.