The old version of prettier we were using didn't support the Flow syntax
to access properties in a type using `SomeType['prop']`. This updates
`prettier` and `rollup-plugin-prettier` to the latest versions.
I added the prettier config `arrowParens: "avoid"` to reduce the diff
size as the default has changed in Prettier 2.0. The largest amount of
changes comes from function expressions now having a space. This doesn't
have an option to preserve the old behavior, so we have to update this.
## Summary
The flag was first added in #16157 and was rolled out to employees in
D17430095. #25997 removed this flag because it wasn't dynamically set to
a value in www. The www side was mistakenly removed in D41851685 due to
deprecation of a TypedJSModule but we still want to keep this flag, so
let's add it back in + add a GK on the www side to match the previous
rollout.
See D42574435 for the dynamic value change in www
## How did you test this change?
```
yarn test
yarn test --prod
```
Revert https://github.com/facebook/react/commit/353c30252. The commit
breaks old React Native where `nativeFabricUIManager` is undefined. I
need to add unit test for this to make sure it doesn't happen in the
future and create a mechanism to deal with undefined
`nativeFabricUIManager`.
This is to unblock React sync to React Native.
This renames Module References to Client References, since they are in
the server->client direction.
I also changed the Proxies exposed from the `node-register` loader to
provide better error messages. Ideally, some of this should be
replicated in the ESM loader too but neither are the source of truth.
We'll replicate this in the static form in the Next.js loaders. cc
@huozhi @shuding
- All references are now functions so that when you call them on the
server, we can yield a better error message.
- References that are themselves already referring to an export name are
now proxies that error when you dot into them.
- `use(...)` can now be used on a client reference to unwrap it server
side and then pass a reference to the awaited value.
## Summary
resolves#26051
After we upgrade to Manifest V3, the browser no longer allow us to run
`eval` within the extension. It's not a problem for prod build, but for
dev build, webpack has been using eval to inject the source map for
devtool. This PR changes it to an alternative method.
I noticed this was an experiment concluded 16 months ago (#21679) that
this extra work is beneficial
to break up cycles leaking memory in product code.
## Summary
Should unblock https://github.com/facebook/react/pull/25970
If the callback for `toWarnDev` was `async` and threw, we didn't
ultimately reject the await Promise from the matcher. This resulted in
tests failing even though the failure was expected due to a test gate.
## How did you test this change?
- [x] tested in https://github.com/facebook/react/pull/25970 with `yarn
test --r=stable --env=development
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js --watch`
- [x] `yarn test`
- [x] CI
This shim is no longer needed on www, in fact I had already deleted it
there and it's currently not on www. See D42503692 which is trying to
add it back as I didn't realize this file was synced from GitHub.
These values are never imported into `ReactFeatureFlags.www.js`, so
they're unused:
- `allowConcurrentByDefault`
- `consoleManagedByDevToolsDuringStrictMode`
These values are never set in the WWW module
(https://fburl.com/code/dsb2ohv8), so they're always `undefined` on www:
- `createRootStrictEffectsByDefault`
- `enableClientRenderFallbackOnTextMismatch`
This mock exists in 2 directories (with identical implementation) and
Jest just picks one at random. This removes one which makes it at least
deterministic and fixes a Jest warning on startup.
It existed in these 2 places:
-
`packages/react-server-dom-relay/src/__mocks__/JSResourceReferenceImpl.js`
-
`packages/react-server-native-relay/src/__mocks__/JSResourceReferenceImpl.js`
(removed)
These suppressions are no longer required.
Generated using:
```sh
flow/tool update-suppressions .
```
followed by adding back 1 or 2 suppressions that were only triggered in
some configurations.
## Summary
I was reading the source code of `ReactFiberLane.js` and I found the
third parameter of the function markRootPinged was not used. So I think
we can remove it.
## How did you test this change?
There is no logic changed, so I think there is no need to add unit
tests. So I run `yarn test` and `yarn test --prod` locally and all tests
are passed.
Co-authored-by: Jan Kassens <jkassens@meta.com>
After the previous changes these upgrade are easy.
- removes config options that were removed
- object index access now requires an indexer key in the type, this
cause a handful of errors that were fixed
- undefined keys error in all places, this needed a few extra
suppressions for repeated undefined identifiers.
Flow's
[CHANGELOG.md](https://github.com/facebook/flow/blob/main/Changelog.md).
This enables the "exact_empty_objects" setting for Flow which makes
empty objects exact instead of building up the type as properties are
added in code below. This is in preparation to Flow 191 which makes this
the default and removes the config.
More about the change in the Flow blog
[here](https://medium.com/flow-type/improved-handling-of-the-empty-object-in-flow-ead91887e40c).
This setting is an incremental path to the next Flow version enforcing
type annotations on most functions (except some inline callbacks).
Used
```
node_modules/.bin/flow codemod annotate-functions-and-classes --write .
```
to add a majority of the types with some hand cleanup when for large
inferred objects that should just be `Fiber` or weird constructs
including `any`.
Suppressed the remaining issues.
Builds on #25918
~~[Fizz] Duplicate completeBoundaryWithStyles to not reference globals~~
## Summary
Follow-up / cleanup PR to #25437
- `completeBoundaryWithStylesInlineLocals` is used by the Fizz external
runtime, which bundles together all Fizz instruction functions (and is
able to reference / rename `completeBoundary` and `resourceMap` as
locals).
- `completeBoundaryWithStylesInlineGlobals` is used by the Fizz inline
script writer, which sends Fizz instruction functions on an as-needed
basis. This version needs to reference `completeBoundary($RC)` and
`resourceMap($RM)` as globals.
Ideally, Closure would take care of inlining a shared implementation,
but I couldn't figure out a zero-overhead inline due to lack of an
`@inline` compiler directive. It seems that Closure thinks that a shared
`completeBoundaryWithStyles` is too large and will always keep it as a
separate function. I've also tried currying / writing a higher order
function (`getCompleteBoundaryWithStyles`) with no luck
## How did you test this change?
- generated Fizz inline instructions should be unchanged
- bundle size for unstable_external_runtime should be slightly smaller
(due to lack of globals)
- `ReactDOMFizzServer-test.js` and `ReactDOMFloat-test.js` should be
unaffected
<!--
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
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
This is the other approach for unifying default and sync lane
https://github.com/facebook/react/pull/25524.
The approach in that PR is to merge default and continuous lane into the
sync lane, and use a new field to track the priority. But there are a
couple places that field will be needed, and it is difficult to
correctly reset the field when there is no sync lane.
In this PR we take the other approach that doesn't remove any lane, but
batch them to get the behavior we want.
## 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
Co-authored-by: Andrew Clark <hi@andrewclark.io>
## Summary
Updating import for babel-code-frame to use the official @babel package,
as babel-code-frame is a ghost dependency. This change is necessary to
avoid potential issues and stay up-to-date with the latest version of
@babel/code-frame, which is already declared in our project's
package.json.
## How did you test this change?
yarn test
Flow introduced a new syntax to annotated the context type of a
function, this tries to update the rest and add 1 example usage.
- 2b1fb91a55 already added the changes
required for eslint.
- Jest transform is updated to use the recommended `hermes-parser` which
can parse current and Flow syntax and will be updated in the future.
- Rollup uses a new plugin to strip the flow types. This isn't ideal as
the npm module is deprecated in favor of using `hermes-parser`, but I
couldn't figure out how to integrate that with Rollup.
## Summary
This PR adds a "perf regression tests" page to react-devtools-shell.
This page is meant to be used as a performance sanity check we will run
whenever we release a new version or finish a major refactor.
Similar to other pages in the shell, this page can load the inline
version of devtools and a test react app on the same page. But this page
does not load devtools automatically like other pages. Instead, it
provides a button that allows us to load devtools on-demand, so that we
can easily compare perf numbers without devtools against the numbers
with devtools.
<img width="561" alt="image"
src="https://user-images.githubusercontent.com/1001890/184059633-e4f0852c-8464-4d94-8064-1684eee626f4.png">
As a first step, this page currently only contain one test:
mount/unmount a large subtree. This is to catch perf issues that
devtools can cause on the react applications it's running on, which was
once a bug fixed in #24863.
In the future, we plan to add:
- more test apps covering different scenarios
- perf numbers within devtools (e.g. initial load)
## How did you test this change?
In order to show this test app can actually catch the perf regression
it's aiming at, I reverted #24863 locally. Here is the result:
https://user-images.githubusercontent.com/1001890/184059214-9c9b308c-173b-4dd7-b815-46fbd7067073.mov
As shown in the video, the time it takes to unmount the large subtree
significantly increased after DevTools is loaded.
For comparison, here is how it looks like before the fix was reverted:
<img width="452" alt="image"
src="https://user-images.githubusercontent.com/1001890/184059743-0968bc7d-4ce4-42cd-b04a-f6cbc078d4f4.png">
## about the `requestAnimationFrame` method
For this test, I used `requestAnimationFrame` to catch the time when
render and commit are done. It aligns very well with the numbers
reported by Chrome DevTools performance profiling. For example, in one
run, the numbers reported by my method are
<img width="464" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060228-990a4c75-f594-411a-9f85-fa5532ec8c37.png">
They are very close to the numbers reported by Chrome profiling:
<img width="456" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060355-a15d1ec5-c296-4016-9c83-03e761f387e3.png">
<img width="354" alt="image"
src="https://user-images.githubusercontent.com/1001890/184060375-19029010-3aed-4a23-890e-397cdba86d9e.png">
`<Profiler>` is not able to catch this issue here.
If you are aware of a better way to do this, please kindly share with
me.
When unwrapping a promise with `use`, we sometimes suspend the work loop
from rendering anything else until the data has resolved. This is
different from how Suspense works in the old throw-a-promise world,
where rather than suspend rendering midway through the render phase, we
prepare a fallback and block the commit at the end, if necessary;
however, the logic for determining whether it's OK to block is the same.
The implementation is only incidentally different because it happens in
two different parts of the code. This means for `use`, we end up doing
the same checks twice, which is wasteful in terms of computation, but
also introduces a risk that the logic will accidentally diverge.
This unifies the implementation by moving it into the SuspenseContext
module. Most of the logic for deciding whether to suspend is already
performed in the begin phase of SuspenseComponent, so it makes sense to
store that information on the stack rather than recompute it on demand.
The way I've chosen to model this is to track whether the work loop is
rendering inside the "shell" of the tree. The shell is defined as the
part of the tree that's visible in the current UI. Once we enter a new
Suspense boundary (or a hidden Offscreen boundary, which acts a Suspense
boundary), we're no longer in the shell. This is already how Suspense
behavior was modeled in terms of UX, so using this concept directly in
the implementation turns out to result in less code than before.
For the most part, this is purely an internal refactor, though it does
fix a bug in the `use` implementation related to nested Suspense
boundaries. I wouldn't be surprised if it happens to fix other bugs that
we haven't yet discovered, especially around Offscreen. I'll add more
tests as I think of them.
This code was originally added in the old ExpirationTime implementation
of Suspense. The idea is that if multiple updates suspend inside the
same Suspense boundary, and both of them resolve, we should render both
results in the same batch, to reduce jank.
This was an incomplete idea, though. We later discovered a stronger
requirement — once we show a fallback, we cannot fill in that fallback
without completing _all_ the updates that were previously skipped over.
Otherwise you get tearing. This was fixed by #18411, then we discovered
additional related flaws that were addressed in #24685. See those PR
descriptions for additional context.
So I believe this older code is no longer necessary.
<!--
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
- Fixes https://github.com/facebook/react/issues/25682
## How did you test this change?
I tried this but it didn't work
```
yarn build --type=UMD_DEV react/index,react-dom && cd fixtures/attribute-behavior && yarn install && yarn start
```
Co-authored-by: eps1lon <silbermann.sebastian@gmail.com>
I found this bug when working on a different task.
`pingSuspendedRoot` sometimes calls `prepareFreshStack` to interupt the
work-in-progress tree and force a restart from the root. The idea is
that if the current render is already in a state where it be blocked
from committing, and there's new data that could unblock it, we might as
well restart from the beginning.
The problem is that this is only safe to do if `pingSuspendedRoot` is
called from a non-React task, like an event handler or a microtask.
While this is usually the case, it's entirely possible for a thenable to
resolve (i.e. to call `pingSuspendedRoot`) synchronously while the
render phase is already executing. If that happens, and work loop
attempts to unwind the stack, it causes the render phase to crash.
Hermes parser is the preferred parser for Flow code going forward. We
need to upgrade to this parser to support new Flow syntax like function
`this` context type annotations or `ObjectType['prop']` syntax.
Unfortunately, there's quite a few upgrades here to make it work somehow
(dependencies between the changes)
- ~Upgrade `eslint` to `8.*`~ reverted this as the React eslint plugin
tests depend on the older version and there's a [yarn
bug](https://github.com/yarnpkg/yarn/issues/6285) that prevents
`devDependencies` and `peerDependencies` to different versions.
- Remove `eslint-config-fbjs` preset dependency and inline the rules,
imho this makes it a lot clearer what the rules are.
- Remove the turned off `jsx-a11y/*` rules and it's dependency instead
of inlining those from the `fbjs` config.
- Update parser and dependency from `babel-eslint` to `hermes-eslint`.
- `ft-flow/no-unused-expressions` rule replaces `no-unused-expressions`
which now allows standalone type asserts, e.g. `(foo: number);`
- Bunch of globals added to the eslint config
- Disabled `no-redeclare`, seems like the eslint upgrade started making
this more precise and warn against re-defined globals like
`__EXPERIMENTAL__` (in rollup scripts) or `fetch` (when importing fetch
from node-fetch).
- Minor lint fixes like duplicate keys in objects.
Depends on #25876
Resubmit #25711 again(previously reverted in #25812), and added the fix
for unwinding in selective hydration during a hydration on the sync
lane.
This PR includes the previously reverted #25695 and #25754, and the fix
for the regression test added in #25867.
Tested internally with a previous failed test, and it's passing now.
Co-authored-by: Andrew Clark <git@andrewclark.io>
We originally had grand plans for using this Event concept for more but
now it's only meant to be used in combination with effects.
It's an Event in the FRP terms, that is triggered from an Effect.
Technically it can also be from another function that itself is
triggered from an existing side-effect but that's kind of an advanced
case.
The canonical case is an effect that triggers an event:
```js
const onHappened = useEffectEvent(() => ...);
useEffect(() => {
onHappened();
}, []);
```
`Offscreen.attach` is imperative API to signal to Offscreen that its
updates should be high priority and effects should be mounted. Coupled
with `Offscreen.detach` it gives ability to manually control Offscreen.
Unlike with mode `visible` and `hidden`, it is developers job to make
sure contents of Offscreen are not visible to users.
`Offscreen.attach` only works if mode is `manual`.
Example uses:
```jsx
let offscreenRef = useRef(null);
<Offscreen mode={'manual'} ref={offscreenRef)}>
<Child />
</Offscreen>
// ------
// Offscreen is attached by default.
// For example user scrolls away and Offscreen subtree is not visible anymore.
offscreenRef.current.detach();
// User scrolls back and Offscreen subtree is visible again.
offscreenRef.current.attach();
```
Co-authored-by: Andrew Clark <git@andrewclark.io>
Add support for `setNativeProps` in Fabric to make migration to the new
architecture easier. The React Native part of this has already landed in
the core and iOS in
1d3fa40c59.
It is still recommended to move away from `setNativeProps` because the
API will not work with future features.
Some tests fail when float is off but when singletons are on. This PR
makes some adjustments
1. 2 singleton tests assert float semantics so will fail.
2. the float dispatcher was being set on the server even when float was
off. while the float calls didn't do anything warnings were still
generated. Instead we provide an empty object for the dispatcher if
float is off. Longer term the dispatcher should move to formatconfig and
just reference the float methods if the flag is on
3. some external fizz runtime tests did not gate against float but
should have
Jan Kassens
