Some build artifacts contain multiple version strings. It seems like an
oversight to me that this `.replace` call just replaces the one that
happens to be first.
(This only affects our own internal repo; it's not a public API.)
I think most of us agree this is a less confusing name. It's possible
someone will confuse it with `console.log`. If that becomes a problem we
can warn in dev or something.
This converts some of our test suite to use the `waitFor` test pattern,
instead of the `expect(Scheduler).toFlushAndYield` pattern. Most of
these changes are automated with jscodeshift, with some slight manual
cleanup in certain cases.
See #26285 for full context.
Over the years, we've gradually aligned on a set of best practices for
for testing concurrent React features in this repo. The default in most
cases is to use `act`, the same as you would do when testing a real
React app. However, because we're testing React itself, as opposed to an
app that uses React, our internal tests sometimes need to make
assertions on intermediate states that `act` intentionally disallows.
For those cases, we built a custom set of Jest assertion matchers that
provide greater control over the concurrent work queue. It works by
mocking the Scheduler package. (When we eventually migrate to using
native postTask, it would probably work by stubbing that instead.)
A problem with these helpers that we recently discovered is, because
they are synchronous function calls, they aren't sufficient if the work
you need to flush is scheduled in a microtask — we don't control the
microtask queue, and can't mock it.
`act` addresses this problem by encouraging you to await the result of
the `act` call. (It's not currently required to await, but in future
versions of React it likely will be.) It will then continue flushing
work until both the microtask queue and the Scheduler queue is
exhausted.
We can follow a similar strategy for our custom test helpers, by
replacing the current set of synchronous helpers with a corresponding
set of async ones:
- `expect(Scheduler).toFlushAndYield(log)` -> `await waitForAll(log)`
- `expect(Scheduler).toFlushAndYieldThrough(log)` -> `await
waitFor(log)`
- `expect(Scheduler).toFlushUntilNextPaint(log)` -> `await
waitForPaint(log)`
These APIs are inspired by the existing best practice for writing e2e
React tests. Rather than mock all task queues, in an e2e test you set up
a timer loop and wait for the UI to match an expecte condition. Although
we are mocking _some_ of the task queues in our tests, the general
principle still holds: it makes it less likely that our tests will
diverge from real world behavior in an actual browser.
In this commit, I've implemented the new testing helpers and converted
one of the Suspense tests to use them. In subsequent steps, I'll codemod
the rest of our test suite.
## Summary
I'm going to start implementing parts of this proposal
https://github.com/react-native-community/discussions-and-proposals/pull/607
As part of that implementation I'm going to refactor a few parts of the
interface between React and React Native. One of the main problems we
have right now is that we have private parts used by React and React
Native in the public instance exported by refs. I want to properly
separate that.
I saw that a few methods to attach event handlers imperatively on refs
were also exposing some things in the public instance (the
`_eventListeners`). I checked and these methods are unused, so we can
just clean them up instead of having to refactor them too. Adding
support for imperative event listeners is in the roadmap after this
proposal, and its implementation might differ after this refactor.
This is essentially a manual revert of #23386.
I'll submit more PRs after this for the rest of the refactor.
## How did you test this change?
Existing jest tests. Will test a React sync internally at Meta.
When resuming a suspended render, there may be more Hooks to be called
that weren't seen the previous time through. Make sure to switch to the
mount dispatcher when calling use() if the next Hook call should be
treated as a mount.
Fixes#25964.
<!--
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
The `yarn flow` command, as suggested for every PR Submission (Task No.
9), tells us `The yarn flow command now requires you to pick a primary
renderer` and provides a list for the same. However, in at the bottom of
the prompt, it suggests `If you are not sure, run yarn flow dom`. This
command `yarn flow dom` does not exist in the list and thus the command
does nothing and exits with `status code 1` without any flow test.
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
While trying to submit a different PR for code cleaning, just during
submission I read the PR Guidelines, and while doing `yarn test`, `yarn
lint`, and `yarn flow`, I came across this issue and thought of
submitting a PR for the same.
## How did you test this change?
Since this code change does not change any logic, just the text
information, I only ran `yarn linc` and `yarn test` for the same.
<!--
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.
-->
Here is how the issue currently looks like:
Signed-off-by: abhiram11 <abhiramsatpute@gmail.com>
Co-authored-by: abhiram11 <abhiramsatpute@gmail.com>
This splits out the Edge and Node implementations of Flight Client into
their own implementations. The Node implementation now takes a Node
Stream as input.
I removed the bundler config from the Browser variant because you're
never supposed to use that in the browser since it's only for SSR.
Similarly, it's required on the server. This also enables generating a
SSR manifest from the Webpack plugin. This is necessary for SSR so that
you can reverse look up what a client module is called on the server.
I also removed the option to pass a callServer from the server. We might
want to add it back in the future but basically, we don't recommend
calling Server Functions from render for initial render because if that
happened client-side it would be a client-side waterfall. If it's never
called in initial render, then it also shouldn't ever happen during SSR.
This might be considered too restrictive.
~This also compiles the unbundled packages as ESM. This isn't strictly
necessary because we only need access to dynamic import to load the
modules but we don't have any other build options that leave
`import(...)` intact, and seems appropriate that this would also be an
ESM module.~ Went with `import(...)` in CJS instead.
## Summary
In rollup v1.19.4, The "treeshake.pureExternalModules" option is
deprecated. The "treeshake.moduleSideEffects" option should be used
instead, see
https://github.com/rollup/rollup/blob/v1.19.4/src/Graph.ts#L130.
## How did you test this change?
ci green
It's confusing to new contributors, and me, that you're supposed to use
`yarn build-combined` for almost everything but not fixtures.
We should use only one build command for everything.
Updated fixtures to use the folder convention of build-combined.
We currently have an awkward set up because the server can be used in
two ways. Either you can have the server code prebundled using Webpack
(what Next.js does in practice) or you can use an unbundled Node.js
server (what the reference implementation does).
The `/client` part of RSC is actually also available on the server when
it's used as a consumer for SSR. This should also be specialized
depending on if that server is Node or Edge and if it's bundled or
unbundled.
Currently we still assume Edge will always be bundled since we don't
have an interceptor for modules there.
I don't think we'll want to support this many combinations of setups for
every bundler but this might be ok for the reference implementation.
This PR doesn't actually change anything yet. It just updates the
plumbing and the entry points that are built and exposed. In follow ups
I'll fork the implementation and add more features.
---------
Co-authored-by: dan <dan.abramov@me.com>
## Summary
I ran into some two factor certification issue and had to resume the
publish script. However, this time if I confirmed the published package,
it will still try to publish the same version and fail. This is not
expected, and it blocks me from publishing the rest of the packages.
## How did you test this change?
I re-run the publish script after the change and successfully publish
the rest of the packages.
```
? Have you run the build-and-test script? Yes
✓ Checking NPM permissions for ryancat. 881 ms
? Please provide an NPM two-factor auth token: 278924
react-devtools version 4.27.2 has already been published.
? Is this expected (will skip react-devtools@4.27.2)? Yes
react-devtools-core version 4.27.2 has already been published.
? Is this expected (will skip react-devtools-core@4.27.2)? Yes
✓ Publishing package react-devtools-inline 23.1 secs
You are now ready to publish the extension to Chrome, Edge, and Firefox:
https://fburl.com/publish-react-devtools-extensions
When publishing to Firefox, remember the following:
Build id: 625690
Git archive: ******
```
## Hoistables
In the original implementation of Float, all hoisted elements were
treated like Resources. They had deduplication semantics and hydrated
based on a key. This made certain kinds of hoists very challenging such
as sequences of meta tags for `og:image:...` metadata. The reason is
each tag along is not dedupable based on only it's intrinsic properties.
two identical tags may need to be included and hoisted together with
preceding meta tags that describe a semantic object with a linear set of
html nodes.
It was clear that the concept of Browser Resources (stylesheets /
scripts / preloads) did not extend universally to all hositable tags
(title, meta, other links, etc...)
Additionally while Resources benefit from deduping they suffer an
inability to update because while we may have multiple rendered elements
that refer to a single Resource it isn't unambiguous which element owns
the props on the underlying resource. We could try merging props, but
that is still really hard to reason about for authors. Instead we
restrict Resource semantics to freezing the props at the time the
Resource is first constructed and warn if you attempt to render the same
Resource with different props via another rendered element or by
updating an existing element for that Resource.
This lack of updating restriction is however way more extreme than
necessary for instances that get hoisted but otherwise do not dedupe;
where there is a well defined DOM instance for each rendered element. We
should be able to update props on these instances.
Hoistable is a generalization of what Float tries to model for hoisting.
Instead of assuming every hoistable element is a Resource we now have
two distinct categories, hoistable elements and hoistable resources. As
one might guess the former has semantics that match regular Host
Components except the placement of the node is usually in the <head>.
The latter continues to behave how the original implementation of
HostResource behaved with the first iteration of Float
### Hoistable Element
On the server hoistable elements render just like regular tags except
the output is stored in special queues that can be emitted in the stream
earlier than they otherwise would be if rendered in place. This also
allow for instance the ability to render a hoistable before even
rendering the <html> tag because the queues for hoistable elements won't
flush until after we have flushed the preamble (`<DOCTYPE
html><html><head>`).
On the client, hoistable elements largely operate like HostComponents.
The most notable difference is in the hydration strategy. If we are
hydrating and encounter a hoistable element we will look for all tags in
the document that could potentially be a match and we check whether the
attributes match the props for this particular instance. We also do this
in the commit phase rather than the render phase. The reason hydration
can be done for HostComponents in render is the instance will be removed
from the document if hydration fails so mutating it in render is safe.
For hoistables the nodes are not in a hydration boundary (Root or
SuspenseBoundary at time of writing) and thus if hydration fails and we
may have an instance marked as bound to some Fiber when that Fiber never
commits. Moving the hydration matching to commit ensures we will always
succeed in pairing the hoisted DOM instance with a Fiber that has
committed.
### Hoistable Resource
On the server and client the semantics of Resources are largely the same
they just don't apply to title, meta, and most link tags anymore.
Resources hoist and dedupe via an `href` key and are ref counted. In a
future update we will add a garbage collector so we can clean up
Resources that no longer have any references
## `<style>` support
In earlier implementations there was no support for <style> tags. This
PR adds support for treating `<style href="..."
precedence="...">...</style>` as a Resource analagous to `<link
rel="stylesheet" href="..." precedence="..." />`
It may seem odd at first to require an href to get Resource semantics
for a style tag. The rationale is that these are for inlining of actual
external stylesheets as an optimization and for URI like scoping of
inline styles for css-in-js libraries. The href indicates that the key
space for `<style>` and `<link rel="stylesheet" />` Resources is shared.
and the precedence is there to allow for interleaving of both kinds of
Style resources. This is an advanced feature that we do not expect most
app developers to use directly but will be quite handy for various
styling libraries and for folks who want to inline as much as possible
once Fizz supports this feature.
## refactor notes
* HostResource Fiber type is renamed HostHoistable to reflect the
generalization of the concept
* The Resource object representation is modified to reduce hidden class
checks and to use less memory overall
* The thing that distinguishes a resource from an element is whether the
Fiber has a memoizedState. If it does, it will use resource semantics,
otherwise element semantics
* The time complexity of matching hositable elements for hydration
should be improved
This is the first of a series of PRs, that let you pass functions, by
reference, to the client and back. E.g. through Server Context. It's
like client references but they're opaque on the client and resolved on
the server.
To do this, for security, you must opt-in to exposing these functions to
the client using the `"use server"` directive. The `"use client"`
directive lets you enter the client from the server. The `"use server"`
directive lets you enter the server from the client.
This works by tagging those functions as Server References. We could
potentially expand this to other non-serializable or stateful objects
too like classes.
This only implements server->server CJS imports and server->server ESM
imports. We really should add a loader to the webpack plug-in for
client->server imports too. I'll leave closures as an exercise for
integrators.
You can't "call" a client reference on the server, however, you can
"call" a server reference on the client. This invokes a callback on the
Flight client options called `callServer`. This lets a router implement
calling back to the server. Effectively creating an RPC. This is using
JSON for serializing those arguments but more utils coming from
client->server serialization.
We're fixing the timing of layout and passive effects in React Native,
and adding support for some Web APIs so common use cases for those
effects can be implemented with the same code on React and React Native.
Let's take this example:
```javascript
function MyComponent(props) {
const viewRef = useRef();
useLayoutEffect(() => {
const rect = viewRef.current?.getBoundingClientRect();
console.log('My view is located at', rect?.toJSON());
}, []);
return <View ref={viewRef}>{props.children}</View>;
}
```
This could would work as expected on Web (ignoring the use of `View` and
assuming something like `div`) but not on React Native because:
1. Layout is done asynchronously in a background thread in parallel with
the execution of layout and passive effects. This is incorrect and it's
being fixed in React Native (see
afec07aca2).
2. We don't have an API to access layout information synchronously. The
existing `ref.current.measureInWindow` uses callbacks to pass the
result. That is asynchronous at the moment in Paper (the legacy renderer
in React Native), but it's actually synchronous in Fabric (the new React
Native renderer).
This fixes point 2) by adding a Web-compatible method to access layout
information (on Fabric only).
This has 2 dependencies in React Native:
1. Access to `getBoundingClientRect` in Fabric, which was added in
https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp#L644-
L676
2. Access to `DOMRect`, which was added in
673c7617bc
.
As next step, I'll modify the implementation of this and other methods
in Fabric to warn when they're accessed during render. We can't do this
on Web because we can't (shouldn't) modify built-in DOM APIs, but we can
do it in React Native because the refs objects are built by the
framework.
## Summary
- yarn.lock diff +-6249, **small pr**
- use jest-environment-jsdom by default
- uncaught error from jsdom is an error object instead of strings
- abortSignal.reason is read-only in jsdom and node,
https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason
## How did you test this change?
ci green
---------
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
We currently abuse the browser builds for Web streams derived
environments. We already have a special build for Bun but we should also
have one for [other "edge"
runtimes](https://runtime-keys.proposal.wintercg.org/) so that we can
maximally take advantage of the APIs that exist on each platform.
In practice, we currently check for a global property called
`AsyncLocalStorage` in the server browser builds which we shouldn't
really do since browsers likely won't ever have it. Additionally, this
should probably move to an import which we can't add to actual browser
builds where that will be an invalid import. So it has to be a separate
build. That's not done yet in this PR but Vercel will follow
Cloudflare's lead here.
The `deno` key still points to the browser build since there's no
AsyncLocalStorage there but it could use this same or a custom build if
support is added.
This updates the Flight fixture to support the new ESM loaders in newer
versions of Node.js.
It also uses native fetch since react-fetch is gone now. (This part
requires Node 18 to run the fixture.)
I also updated everything to use the `"use client"` convention instead
of file name based convention.
The biggest hack here is that the Webpack plugin now just writes every
`.js` file in the manifest. This needs to be more scoped. In practice,
this new convention effectively requires you to traverse the server
graph first to find the actual used files. This is enough to at least
run our own fixture though.
I didn't update the "blocks" fixture.
More details in each commit message.
The "dom" configuration is actually the node specific configuration. It
just happened to be that this was the mainline variant before so it was
implied but with so many variants, this is less obvious now.
The "bun" configuration is specifically for "bun". There's no "native"
renderer for "bun" yet.
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.
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
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.
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
## 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.
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.
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();
}, []);
```
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.
