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.
Follow up to #26827.
These can't include binary data and we don't really have any use cases
that really require these to already be strings.
When the stream is encoded inside another protocol - such as HTML we
need a different format that encode binary offsets and binary data.
## Summary
This is required for the case when we have an instance and want to get
inspector data for it. Such case occurs when RN's application being
debugged via React DevTools.
React DevTools sends instance to RN, which then gets all auxiliary data
to highlight some elements. Having `getInspectorDataForInstance` method
exposed makes it possible to easily get current props from fiber, which
then can be used to display some margins & paddings for hovered element
(via props.style).
I see that `getInspectorDataForInstance` is being exported at the top
level of the renderer, but feels like this should also be inside
DevTools global hook, the same way we use it for
[`getInspectorDataForViewAtPoint`](https://github.com/facebook/react-native/blob/e7d3662904e0e35e43380ee6d54859388713a592/packages/react-native/Libraries/Inspector/getInspectorDataForViewAtPoint.js).
We currently support passing an XHR request to Flight for broader compat
and possibly better perf than `fetch()`. However, it's a little tricky
because ideally the RSC protocol is really meant to support binary data
too. XHR does support binary but it doesn't support it while also
streaming.
We could maybe support this only when you know it's going to be only
text streams but it has some limitations in how we can encode separators
if we can't use binary.
Nobody is really asking for this so we might as well delete it.
This isn't really meant to be actually used, there are many issues with
this approach, but it shows the capabilities as a proof-of-concept.
It's a new reference implementation package `react-server-dom-esm` as
well as a fixture in `fixtures/flight-esm` (fork of `fixtures/flight`).
This works pretty much the same as pieces we already have in the Webpack
implementation but instead of loading modules using Webpack on the
client it uses native browser ESM.
To really show it off, I don't use any JSX in the fixture and so it also
doesn't use Babel or any compilation of the files.
This works because we don't actually bundle the server in the reference
implementation in the first place. We instead use [Node.js
Loaders](https://nodejs.org/api/esm.html#loaders) to intercept files
that contain `"use client"` and `"use server"` and replace them. There's
a simple check for those exact bytes, and no parsing, so this is very
fast.
Since the client isn't actually bundled, there's no module map needed.
We can just send the file path to the file we want to load in the RSC
payload for client references.
Since the existing reference implementation for Node.js already used ESM
to load modules on the server, that all works the same, including Server
Actions. No bundling.
There is one case that isn't implemented here. Importing a `"use
server"` file from a Client Component. We don't have that implemented in
the Webpack reference implementation neither - only in Next.js atm. In
Webpack it would be implemented as a Webpack loader.
There are a few ways this can be implemented without a bundler:
- We can intercept the request from the browser importing this file in
the HTTP server, and do a quick scan for `"use server"` in the file and
replace it just like we do with loaders in Node.js. This is effectively
how Vite works and likely how anyone using this technique would have to
support JSX anyway.
- We can use native browser "loaders" once that's eventually available
in the same way as in Node.js.
- We can generate import maps for each file and replace it with a
pointer to a placeholder file. This requires scanning these ahead of
time which defeats the purposes.
Another case that's not implemented is the inline `"use server"` closure
in a Server Component. That would require the existing loader to be a
bit smarter but would still only "compile" files that contains those
bytes in the fast path check. This would also happen in the loader that
already exists so wouldn't do anything substantially different than what
we currently have here.
Currently we preload all scripts that are not hoisted. One of the
original reasons for this is we stopped SSR rendering async scripts that
had an onLoad/onError because we needed to be able to distinguish
between Float scripts and non-Float scripts during hydration. Hydration
has been refactored a bit and we can not get around this limitation so
we can just emit the async script in place. However, sync and defer
scripts are also preloaded. While this is sometimes desirable it is not
universally so and there are issues with conveying priority properly
(see fetchpriority) so with this change we remove the automatic
preloading of non-Float scripts altogether.
For this change to make sense we also need to emit async scripts with
loading handlers during SSR. we previously only preloaded them during
SSR because it was necessary to keep async scripts as unambiguously
resources when hydrating. One ancillary benefit was that load handlers
would always fire b/c there was no chance the script would run before
hydration. With this change we go back to having the ability to have
load handlers fired before hydration. This is already a problem with
images and we don't have a generalized solution for it however our
likely approach to this sort of thing where you need to wait for a
script to load is to use something akin to `importScripts()` rather than
rendering a script with onLoad.
We previously preloaded stylesheets that were rendered in Fizz. The idea
was we'd get a headstart fetching these resources since we know they are
going to be rendered. However to really be effective non-float
stylesheets need to rendered in the head and the preload here is not
helpful and potentially hurtful to perf in a minor way. This change
removes this functionality to make the code smaller and simpler
<!--
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?
-->
In StrictMode, React currently only triggers `componentWillUnmount` if
`componentDidMount` is defined. This would miss detecting issues like
initializing resources in constructor or componentWillMount, for
example:
```
class Component {
constructor() {
this._subscriptions = new Subscriptions();
}
componentWillUnmount() {
this._subscriptions.reset();
}
}
```
The PR makes `componentWillUnmount` always run in StrictMode.
## 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.
-->
`yarn test ci`
## Overview
Does a few things:
- Renames `enableSyncDefaultUpdates` to
`forceConcurrentByDefaultForTesting`
- Changes the way it's used so it's dead-code eliminated separate from
`allowConcurrentByDefault`
- Deletes a bunch of the gated code
The gates that are deleted are unnecessary now. We were keeping them
when we originally thought we would come back to being concurrent by
default. But we've shifted and now sync-by default is the desired
behavior long term, so there's no need to keep all these forked tests
around.
I'll follow up to delete more of the forked behavior if possible.
Ideally we wouldn't need this flag even if we're still using
`allowConcurrentByDefault`.
stacked on #26753
Adds support for preloading bootstrapModules. We don't yet support
modules in Float's public interface but this implementation should be
compatible with what we do when we add it.
This PR adds a preload for bootstrapScripts. preloads are captured
synchronously when you create a new Request and as such the normal logic
to check if a preload already exists is skipped.
clearContainer and clearSingleton both assumed scripts could be safely
removed from the DOM because normally once a script has been inserted
into the DOM it is executable and removing it, even synchronously, will
not prevent it from running. However There is an edge case in a couple
browsers (Chrome at least) where during HTML streaming if a script is
opened and not yet closed the script will be inserted into the document
but not yet executed. If the script is removed from the document before
the end tag is parsed then the script will not run. This change causes
clearContainer and clearSingleton to retain script elements. This is
generally thought to be safe because if we are calling these methods we
are no longer hydrating the container or the singleton and the scripts
execution will happen regardless.
This solves an issue where if you inject a hidden field in the beginning
of the form, we might mistakenly hydrate the injected one that was part
of an action.
I'm not too happy about how specific this becomes. It's similar to Float
but in general we don't do this deep comparison.
See https://github.com/vercel/next.js/issues/50087
There was a CircleCI bug that prevented the sizebot job from accessing
the artifacts API. I had added a temporary workaround to pull from a
mirror instead. This seems to have been fixed, so I can remove the
workaround.
With df12d7eac4 I accidentally made it so
that tests aren't run with the 2 variant modes for most
SchedulerFeatureFlags anymore. This fixes it with the same approach as
ee4233bdbc.
Test Plan:
Run and notice the boolean flags follow the variant:
```
yarn test-www --variant=true
yarn test-www --variant=false
```
Currently when React generates rel=preload link tags for script/stylesheet resources, it will not carryover nonce and fetchpriority values if specified on the original elements.
This change ensures that the preload links use the nonce and fetchPriority values if they were specified.
…affiliates.
## Summary
There were 8 different places where the copyright comment was wrong.
Rewrote from "Copyright (c) Facebook, Inc. and its affiliates." to
"Copyright (c) Meta Platforms, Inc. and its affiliates."
## How did you test this change?
No code was changed. Comment was still a comment after changes.
Co-authored-by: Dennis Moradkhani <denmo530@student.liu.se>
## Summary
Changed the comment in react/packages/react
/react.shared-subset.js saying
```
Copyright (c) Facebook, Inc. and affiliates ..
```
To
```
Copyright (c) Meta Platforms, Inc. and affiliates ..
```
as raised in the following issues:
https://github.com/facebook/react/issues/26829
Files Changed:
react/packages/react/react.shared-subset.js
## How did you test this change?
Tests Required: No
The bindings upstream in Relay has been removed so we don't need these
builds anymore. The idea is to revisit an FB integration of Flight but
it wouldn't use the Relay specific bindings. It's a bit unclear how it
would look but likely more like the OSS version so not worth keeping
these around.
The `dom-relay` name also included the FB specific Fizz implementation
of the streaming config so I renamed that to `dom-fb`. There's no Fizz
implementation for Native yet so I just removed `native-relay`.
We created a configurable fork for how to encode the output of Flight
and the Relay implementation encoded it as JSON objects instead of
strings/streams. The new implementation would likely be more stream-like
and just encode it directly as string/binary chunks. So I removed those
indirections so that this can just be declared inline in
ReactFlightServer/Client.
Sebastian Markbåge
Andrew Clark
Noah Lemen
Ruslan Lesiutin
Tianyu Yao
Jan Kassens
Josh Story
Henrique Limas
Jack Works
BIKI DAS
Ricky
Samuel Susla
Yen-Wei Liu
Dennis Moradkhani
Sanket Singh
Ibrahem Mahyob