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.
While the goal is to remove legacy context completely, I think we can
already land the removal of legacy context for function components. I
didn't even know this feature existed until reading the code recently.
The win is just a couple of property lookups on function renders, but it
trims down the API already as the full removal will likely still take a
bit more time.
www: Starting with enabled test renderer and a feature flag for
production rollout.
RN: Not enabled, will follow up on this.
This avoids potential differences between Git versions.
Having a guarantee unique hash isn't neccessary as it's just
informational and we have the date in the version string as well.
This code is getting deleted in #30313 anyway but it should've been
gated all along.
This code exists to basically manually transpile console.error to
consoleWithStackDev because the transpiler doesn't work on `.apply` or
`.bind` or the dynamic look up. We only apply the transform in DEV so we
should've only done this in DEV.
Otherwise these logs get silenced in prod.
<!--
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?
-->
Now that HostContext determination for Fabric is a DEV-only behavior, we
can move the HostContext determination to resolve from the ViewConfig
for a given type. Doing this will allow arbitrary types to register
themselves as potential parents of raw text string children. This is the
first of two diffs for react as we'll:
1. Add the new property to the ViewConfig types
2. Update React Native to include the `supportsRawText` property for
`RCTText`, `RCTVirtualText`, `AndroidTextInput`, etc.
3. Switch the behavior of react to read from the ViewConfig rather than
a static list of types.
## 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
- yarn test --prod
- Pulled change into react-native, added `supportsRawText` props to
RCTText, RCTVirtualText, etc. ViewConfigs and confirmed everything type
checks.
I'm pretty sure this is completely unnecessary even in www and RN
because it's only useful if you use the mock scheduler which typically
only we do in our own tests. But all our tests pass so unless www/RN
does something with it, I don't think this is used.
Also remove unnecessary `__DEV__` check. If it gets pulled in prod, we'd
want to know about it.
Object literals should be faster at least on React Native with Hermes as
the JS engine.
It might also be interesting to confirm the old comments in this file
from years ago are even still valid. Creating an object from a literal
should be a simpler operation.
It's a bit unfortunate that this introduces a bunch of copied code, but
since we rearely update the fields on fibers, this seems like an okay
tradeoff for a hot code path. An alternative would be some sort of macro
system, but that doesn't seem worth the extra complexity.
We're removing this wrapper from the mainline but RN is still using
component stacks to filter out warnings.
This is unfortunate since it'll be hard to keep track of the interplay
with these, DevTools and how you're supposed to implement error dialogs
in userspace.
Currently we're printing parent stacks at the end of DOM nesting even
with owner stacks enabled. That's because the context of parent tree is
relevant for determining why two things are nested. It might not be
sufficient to see the owner stack alone.
I'm trying to get rid of parent stacks and rely on more of the plain
owner stacks or ideally console.createTask. These are generally better
anyway since the exact line for creating the JSX is available. It also
lets you find a parent stack frame that is most relevant e.g. if it's
hidden inside internals.
For DOM nesting there's really only two stacks that are relevant. The
creation of the parent and the creation of the child. Sometimes they're
close enough to be the same thing. Such as for parents that can't have
text children or when the ancestor is the direct parent created at the
same place (same owner).
Sometimes they're far apart. In this case I add a second console.error
within the context of the ancestor. That way the second stack trace can
be used to read the stack trace for where it was created.
To preserve some parent context I now print the parent stack in a diff
view format using the logic from hydration diffs. This includes some
siblings and props for context.
<img width="756" alt="Screenshot 2024-07-10 at 12 21 38 AM"
src="https://github.com/facebook/react/assets/63648/0843133d-cc7a-4ecc-91c0-f46ae8e99f20">
Text Nodes:
<img width="749" alt="Screenshot 2024-07-10 at 12 37 40 AM"
src="https://github.com/facebook/react/assets/63648/ee377d82-54ee-450a-99d1-fcc3ef290d59">
---------
Co-authored-by: tjallingt <tjallingt@gmail.com>
For environments that still have legacy contexts available, this adds a
warning to make the remaining call sites easier to locate and encourage
upgrades.
This marker can then be emitted as a getter. When this object gets
accessed we use a special error to let the user know what is going on.
<img width="1350" alt="Screenshot 2024-07-08 at 10 13 46 PM"
src="https://github.com/facebook/react/assets/63648/e3eb698f-e02d-4394-a051-ba9ac3236480">
When you click the `...`:
<img width="1357" alt="Screenshot 2024-07-08 at 10 13 56 PM"
src="https://github.com/facebook/react/assets/63648/4b8ce1cf-d762-49a4-97b9-aeeb1aa8722c">
I also increased the object limit in console logs. It was arbitrarily
set very low before.
These limits are per message. So if you have a loop of many logs it can
quickly add up a lot of strain on the server memory and the client. This
is trying to find some tradeoff. Unfortunately we don't really do much
deduping in these logs so with cyclic objects it ends up maximizing the
limit and then siblings aren't logged.
Ideally we should be able to lazy load them but that requires a lot of
plumbing to wire up so if we can avoid it we should try to. If we want
to that though one idea is to use the getter to do a sync XHR to load
more data but the server needs to retain the objects in memory for an
unknown amount of time. The client could maybe send a signal to retain
them until a weakref clean up but even then it kind of needs a heartbeat
to let the server know the client is still alive. That's a lot of
complexity. There's probably more we can do to optimize deduping and
other parts of the protocol to make it possible to have even higher
limits.
Only for parent stacks. This ensures that we can use the regular
mechanism for appending stack traces. E.g. you can polyfill it.
This is mainly so that I can later remove the automatic appending.
Component stacks have a similar problem to the problem with keyPath
where we had to move it down and set it late right before recursing.
Currently we work around that by popping exactly one off when something
suspends. That doesn't work with the new server stacks being added which
are more than one. It also meant that we kept having add a single frame
that could be popped when there shouldn't need to be one.
Unlike keyPath component stacks has this weird property that once
something throws we might need the stack that was attempted for errors
or the previous stack if we're going to retry and just recreate it.
I've tried a few different approaches and I didn't like either but this
is the one that seems least problematic.
I first split out renderNodeDestructive into a retryNode helper. During
retries only retryNode is called. When we first discover a node, we pass
through renderNodeDestructive.
Instead of add a component stack frame deep inside renderNodeDestructive
after we've already refined a node, we now add it before in
renderNodeDestructive. That way it's only added once before being
attempted. This is similar to how Fiber works where in ChildFiber we
match the node once to create the instance and then later do we attempt
to actually render it and it's only the second part that's ever retried.
This unfortunately means that we now have to refine the node down to
element/lazy/thenables twice. To avoid refining the type too I move that
to be done lazily.
We already added this for other thrown errors, not just console.errors.
There's a production form of this. We just missed adding this context.
Mainly the best context is the line number though which comes from owner
stacks.
Before:
<img width="844" alt="Screenshot 2024-07-04 at 3 20 34 PM"
src="https://github.com/facebook/react/assets/63648/0fd8a53f-538a-4429-a4cf-c22f85a09aa8">
After:
<img width="845" alt="Screenshot 2024-07-05 at 6 08 28 PM"
src="https://github.com/facebook/react/assets/63648/7b9da13a-fa97-4581-9899-06de6fface65">
Firefox:
<img width="1338" alt="Screenshot 2024-07-05 at 6 09 50 PM"
src="https://github.com/facebook/react/assets/63648/f2eb9f2a-2251-408f-86d0-b081279ba378">
The first log doesn't get a stack because it's logged before DevTools
boots up and connects which is unfortunate.
The second log already has a stack printed by React (this is on stable)
it gets replaced by our object now.
The third and following logs don't have a stack and get one appended.
I only turn the stack into an error object if it matches what we would
emit from DevTools anyway. Otherwise we assume it's not React. Since I
had to change the format slightly to make this work, I first normalize
the stack slightly before doing a comparison since it won't be 1:1.
This was missed in the mount dev dispatcher. It was only in the rerender
dispatcher which means that it was only logged during the rerender.
Since DevTools can hide logs during rerenders, this hid the warning in
StrictMode.
Joe Savona
Mofei Zhang
Jan Kassens
Dmytro Rykun
Sebastian Markbåge
Josh Story
Jelle Voost
Lauren Tan
Eric Rozell
Niklas Mollenhauer