Build artifacts are uniquely associated to a single workflow run, so
appending the sha was unnecessary. I originally included it to make it
easier to download later but this turns out to be unneeded.
Drops the sha suffix to make downloading the artifact in a separate
script / workflow more straightforward.
ghstack-source-id: 36ac4df4c3
Pull Request resolved: https://github.com/facebook/react/pull/30364
Addresses a follow-up from the previous PR. Destructured function params are currently not eagerly promoted to temporaries: we wait until PromotedUsedTemporaries. But params _always_ have to be named, so we can promote when constructing HIR.
ghstack-source-id: a6f665762e
Pull Request resolved: https://github.com/facebook/react/pull/30332
Implements general-purpose function outlining. Specifically, anonymous function expressions which have no dependencies/context variables are extracted into named top-level functions. The original function expression is replaced with a `LoadGlobal` of the generated name.
Note that the architecture is designed to allow very general purpose forms of outlining, though we currently are very conservative in what we outline. Specifically, the outlining allows annotating functions with an optional ReactiveFunctionType, which if set will cause the outlined function to get compiled as that type. So we could for example outline a helper hook or helper component, set the type, and then have the hook/component get memoized as well. For now though we just outline with no type set, and generate the function as-is without running it through compilation.
ghstack-source-id: 2a7da6c8e8
Pull Request resolved: https://github.com/facebook/react/pull/30331
Refactors Program.ts to first traverse the `Program` node and build up a queue of functions to visit, then iterate that queue and compile the functions. This doesn't change behavior, but allows the next diff to add additional items to the queue during compilation (for function outlining).
ghstack-source-id: 858527c30c
Pull Request resolved: https://github.com/facebook/react/pull/30330
Stores the Babel `Scope` object for the current function on the Environment, allowing access later for generating new globally unique names. The idea is to expose a small subset of the capabilities of the Scope API via Environment, so that the rest of the compiler remains decoupled from Babel. Ideally we'd use our own Scope implementation too, but we can punt on that for now since the parts we're using (global id generation) seem pretty reliable.
ghstack-source-id: 37f7113b11
Pull Request resolved: https://github.com/facebook/react/pull/30328
Now that these run in GH, we can remove these jobs from the circleci
build_and_test workflow to speed up the remaining jobs left in circleci
since this was the long pole.
I have left the definition of these jobs in tact however, as they are
used for devtools_regression_tests which has yet to be migrated to GH.
ghstack-source-id: 6f4ed3efa2
Pull Request resolved: https://github.com/facebook/react/pull/30352
The tests for the build currently take about 3.5 minutes and is at the
moment the long pole for the whole workflow. By sharding we can make
this workflow faster and save about a minute or so in overall wall time.
ghstack-source-id: c23df7b6d7
Pull Request resolved: https://github.com/facebook/react/pull/30349
Safari has a behavior (bug) where when you consturct an Image in
javascript if you set srcset before properties for `sizes` the brwoser
will download the largest image size because it starts to load before
you communicate the sizes information.
https://x.com/OliverJAsh/status/1812408504444989588?t=CVHPqBaUiF5-6DBPGERTDA
There are likely other combinations or property order assignment that
can cause problems such as setting crossorigin after assigning src or
srcset. Conceptually we should withold the src and srcSet from the Image
instance until last so all relevant other properties can be assigned
before actually initiating any network activity.
This is an unforunate amount of code for what is realistically a bug in
Browsers but it should allow us to avoid weird regressions depending on
prop object order.
I didn't change the preload prop order because I don't believe preload
links have the same issue (they are not fetched as eagerly I believe).
One nice benefit of this change though is the img case can move higher
in the switch which is likely optimal given it's a relatively common
tag. Previously it was as low as it was because it was part of the void
element set so it couldn't be elevated without elevating less common
tags
---------
Co-authored-by: Jan Kassens <jan@kassens.net>
I'm experimenting with a new pass that sometimes creates scopes with early returns earlier in the pipeline, but there are a few passes that assume that can't happen. This PR is updating those passes just to be more resilient to help unblock experimentation.
ghstack-source-id: a9e348181d
Pull Request resolved: https://github.com/facebook/react/pull/30333
We have an experimental mode where we generate scopes for simple phi values, even if they aren't subsequently mutated. This mode was incorrectly generating scope ranges, leaving the start at 0 which is invalid. The fix is to allow non-zero identifier ranges to overwrite the scope start (rather than taking the min) if the scope start is still zero.
ghstack-source-id: ecbb04c96e
Pull Request resolved: https://github.com/facebook/react/pull/30321
---
The current version of `@babel/generator` used by playground has some bugs (see https://github.com/babel/babel/issues/10966)
```js
// Try pasting this into playground
function useFoo(a, b) {
return (a ?? b) == c;
}
// Current playground output
function useFoo(a, b) {
return a ?? b == c;
}
```
We previously locked babel library versions to be compatible with the oldest Meta internal usages. Now that both compiler and eslint plugins are bundled with rollup, this shouldn't be necessary.
ghstack-source-id: fa20d676b5
Pull Request resolved: https://github.com/facebook/react/pull/30341
---
* panicThreshold: `all_errors` -> `none`
* inject an error logger through compiler config (instead of using exceptions)
We currently report at most one lint warning per file, this lets us exhaustively report all available ones (see new
test fixture for example)
ghstack-source-id: 5299315574
Pull Request resolved: https://github.com/facebook/react/pull/30336
## Summary
This PR fixes the `fastAddProperties` function. Now it nullifies a prop
if it was defined in one of the items of a style array, but then set to
`undefined` or `null` in one of the subsequent items. E.g. `style:
[{top: 0}, {top: undefined}]` should evaluate to `{top: null}`. Also
added a test case for that.
## How did you test this change?
```
yarn test packages/react-native-renderer -r=xplat --variant=false
yarn test packages/react-native-renderer -r=xplat --variant=true
yarn flow native
```
Unfortunately, Firefox doesn't include the name of a function in stack
traces if you set it as either `.name` or `.displayName` at runtime.
Only if you include it declarative.
We also can't include it into a named function expression because not
all possible names are expressible declaratively. E.g. spaces or
punctuations.
However, we can express any name if it's an object property and since
object properties now give their name declarative to the function
defined inside of them, we can declaratively express any name this way.
The interstital characters in our link header tracking are not
contributing to the remaining capacity calculation so when a lot of
inditidual links are present in the link header it can allow an
overflowing link header to be included. This change corrects the math so
it properly prevents overflow.
## Summary
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
A proposed fix for the bug described in
https://github.com/facebook/react/issues/25967
## How did you test this change?
See the issue linked above, test scenario included in the code sandbox:
https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx
<!--
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.
-->
Stacked on #30308.
This is now a noop module so we can stop applying the transform of
console.error using the Babel plugin in the mainline builds. I'm keeping
the transform for RN/WWW for now although it might be nice if the
transform moved into those systems as it gets synced instead of keeping
it upstream.
In jest tests we're already not running the forks for RN/WWW so we don't
need it at all there.
React transpiles some of its own `console.error` calls into a helper
that appends component stacks to those calls. However, this doesn't
cover user space `console.error` calls - which includes React helpers
that React has moved into third parties like createClass and prop-types.
The idea is that any user space component can add a warning just like
React can which is why React DevTools adds them too if they don't
already exist. Having them appended in both places is tricky because now
you have to know whether to remove them from React's logs.
Similarly it's often common for server-side frameworks to forget to
cover the `console.error` logs from other sources since React DevTools
isn't active there. However, it's also annoying to get component stacks
clogging the terminal - depending on where the log came from.
In the future `console.createTask()` will cover this use case natively
and when available we don't append them at all.
The new strategy relies on either:
- React DevTools existing to add them to React logs as well as third
parties.
- `console.createTask` being supported and surfaced.
- A third party framework showing the component stack either in an Error
Dialog or appended to terminal output.
For a third party to be able to implement this they need to be able to
get the component stack. To get the component stack from within a
`console.error` call you need to use the `React.captureOwnerStack()`
helper which is only available in `enableOwnerStacks` flag. However,
it's possible to polyfill with parent stacks using internals as a stop
gap. There's a question of whether React 19 should just go out with
`enableOwnerStacks` to expose this but regardless I think it's best it
doesn't include component stacks from the runtime for consistency.
In practice it's not really a regression though because typically either
of the other options exists and error dialogs don't implement
`console.error` overrides anyway yet. SSR terminals might miss them but
they'd only have them in DEV warnings to begin with an a subset of React
warnings. Typically those are either going to happen on the client
anyway or replayed.
Our tests are written to assert that component stacks work in various
scenarios all over the place. To ensure that this keeps working I
implement a "polyfill" that is similar to that expected a server
framework might do - in `assertConsoleErrorDev` and `toErrorDev`.
This PR doesn't yet change www or RN since they have their own forks of
consoleWithStackDev for now.
Merges most workflows into a single workflow, which should make the
"Checks" tab easier to visualize sincee it will all be graphed in a
single interface. There should be no change in behavior, this is a
mechanical change.
ghstack-source-id: a5241a2420
Pull Request resolved: https://github.com/facebook/react/pull/30324
Quick change to standardize on a single timezone across all workflows
and to use the same version of node (18.20.1).
Also updates .nvmrc
ghstack-source-id: e1d43006ec
Pull Request resolved: https://github.com/facebook/react/pull/30323
In www, the experimental versions get a .modern.js or .classic.js prefix
and get copied into the same folder. In RN, they don't seem to have
.modern.js and .classic.js versions so they end up getting the same
name.
sebmarkbage's theory is that what happens is that they then override
the file that was already there. So depending on if experimental or
stable build finishes first you get a different version at the end.
It doesn't make sense to use `__EXPERIMENTAL__` for flags in native-fb
since there's no modern/classic split there. So that flag should just be
hardcoded to true or false and then it doesn't matter which one finishes
first.
We don't support experimental builds in OSS RN neither so the same thing
could happen with
[`enableOwnerStacks`](https://github.com/facebook/react/blob/5dcf3ca8d45a276a8b4cee0cedd234967661ca35/packages/shared/forks/ReactFeatureFlags.native-oss.js#L60).
You can see that the build errors in the previous PR but passes after
these flag changes.
ghstack-source-id: d10f37bcea
Pull Request resolved: https://github.com/facebook/react/pull/30322
I noticed that the www-modern builds pick up the `.experimental.js`
entry points but these flags that are associated with these exports are
not enabled in www, so it gets the wrong builds.
This file is just a clone over the stable `ReactServer.js` one. We
should probably do the reexport pattern instead.
The full stack is the current execution stack (`new Error().stack`) +
the current owner stack (`React.captureOwnerStack()`).
The idea with the top frame was that when we append it to console.error
we'd include both since otherwise the true reason would be obscured
behind the little `>` to expand. So we'd just put both stack front and
center. By adding this into getCurrentStack it was easy to use the same
filtering. I never implemented in Fizz or Flight though.
However, with the public API `React.captureOwnerStack()` it's not
necessary to include the current stack since you already have it and
you'd have filtering capabilities in user space too.
Since I'm removing the component stacks from React itself we no longer
need this. It's expected that maybe RDT or framework polyfill would
include this same technique though.
Mofei Zhang
Lauren Tan
Joe Savona
Niklas MollenhauerandGitHub
Jan KassensandGitHub
Dmytro RykunandGitHub
Sebastian MarkbågeandGitHub
Jelle VoostandGitHub