Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Track the Real Fiber for Key Warnings #29791

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jun 6, 2024

This refactors key warning to happen inline after we've matched a Fiber. I didn't want to do that originally because it was riskier. But it turns out to be straightforward enough.

This lets us use that Fiber as the source of the warning which matters to DevTools because then DevTools can associate it with the right component after it mounts.

We can also associate the duplicate key warning with this Fiber. That way we'll get the callsite with the duplicate key on the stack and can associate this warning with the child that had the duplicate.

I kept the forked DevTools tests because the warning now is counted on the Child instead of the Parent (18 behavior).

However, this won't be released in 19.0.0 so I only test this in whatever the next version is.

Doesn't seem worth it to have a test for just the 19.0.0 behavior.

This lets us use that Fiber as the source of the warning which matters
to DevTools because then DevTools can associate it with the right
component.
Copy link

vercel bot commented Jun 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 11:05pm

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 6, 2024
@react-sizebot
Copy link

react-sizebot commented Jun 6, 2024

Comparing: c4b433f...8b54f40

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.26 kB 497.25 kB = 89.11 kB 89.11 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.08 kB 502.07 kB = 89.80 kB 89.80 kB
facebook-www/ReactDOM-prod.classic.js = 594.56 kB 594.54 kB = 104.72 kB 104.71 kB
facebook-www/ReactDOM-prod.modern.js = 570.95 kB 570.93 kB = 101.13 kB 101.13 kB
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Generated by 🚫 dangerJS against 8b54f40

That way we'll get the callsite with the duplicate key on the stack
The warning now is counted on the Child instead of the parent.
However, this won't be released in 19.0 so I only test this in whatever
the next version is.
@sebmarkbage sebmarkbage merged commit a0a435d into facebook:main Jun 7, 2024
44 checks passed
hoxyq added a commit that referenced this pull request Jun 18, 2024
Full list of changes:

* chore[react-devtools]: improve console arguments formatting before
passing it to original console ([hoxyq](https://github.com/hoxyq) in
[#29873](#29873))
* chore[react-devtools]: unify console patching and default to ansi
escape symbols ([hoxyq](https://github.com/hoxyq) in
[#29869](#29869))
* chore[react-devtools/backend]: remove
consoleManagedByDevToolsDuringStrictMode
([hoxyq](https://github.com/hoxyq) in
[#29856](#29856))
* chore[react-devtools/extensions]: make source maps url relative
([hoxyq](https://github.com/hoxyq) in
[#29886](#29886))
* fix[react-devtools] divided inspecting elements between inspecting do…
([vzaidman](https://github.com/vzaidman) in
[#29885](#29885))
* [Fiber] Create virtual Fiber when an error occurs during reconcilation
([sebmarkbage](https://github.com/sebmarkbage) in
[#29804](#29804))
* fix[react-devtools] component badge in light mode is now not invisible
([vzaidman](https://github.com/vzaidman) in
[#29852](#29852))
* Remove Warning: prefix and toString on console Arguments
([sebmarkbage](https://github.com/sebmarkbage) in
[#29839](#29839))
* Add jest lint rules ([rickhanlonii](https://github.com/rickhanlonii)
in [#29760](#29760))
* [Fiber] Track the Real Fiber for Key Warnings
([sebmarkbage](https://github.com/sebmarkbage) in
[#29791](#29791))
* fix[react-devtools/store-test]: fork the test to represent current be…
([hoxyq](https://github.com/hoxyq) in
[#29777](#29777))
* Default native inspections config false
([vzaidman](https://github.com/vzaidman) in
[#29784](#29784))
* fix[react-devtools] remove native inspection button when it can't be
used ([vzaidman](https://github.com/vzaidman) in
[#29779](#29779))
* chore[react-devtools]: ip => internal-ip
([hoxyq](https://github.com/hoxyq) in
[#29772](#29772))
* Fix #29724: `ip` dependency update for CVE-2024-29415
([Rekl0w](https://github.com/Rekl0w) in
[#29725](#29725))
* cleanup[react-devtools]: remove unused supportsProfiling flag from
store config ([hoxyq](https://github.com/hoxyq) in
[#29193](#29193))
* [Fiber] Enable Native console.createTask Stacks When Available
([sebmarkbage](https://github.com/sebmarkbage) in
[#29223](#29223))
* Move createElement/JSX Warnings into the Renderer
([sebmarkbage](https://github.com/sebmarkbage) in
[#29088](#29088))
* Set the current fiber to the source of the error during error
reporting ([sebmarkbage](https://github.com/sebmarkbage) in
[#29044](#29044))
* Unify ReactFiberCurrentOwner and ReactCurrentFiber
([sebmarkbage](https://github.com/sebmarkbage) in
[#29038](#29038))
* Dim `console` calls on additional Effect invocations due to
`StrictMode` ([eps1lon](https://github.com/eps1lon) in
[#29007](#29007))
* refactor[react-devtools]: rewrite context menus
([hoxyq](https://github.com/hoxyq) in
[#29049](#29049))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants