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

Move createElement/JSX Warnings into the Renderer #29088

Merged
merged 9 commits into from
May 23, 2024

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented May 16, 2024

This is necessary to simplify the component stack handling to make way for owner stacks. It also solves some hacks that we used to have but don't quite make sense. It also solves the problem where things like key warnings get silenced in RSC because they get deduped. It also surfaces areas where we were missing key warnings to begin with.

Almost every type of warning is issued from the renderer. React Elements are really not anything special themselves. They're just lazily invoked functions and its really the renderer that determines there semantics.

We have three types of warnings that previously fired in JSX/createElement:

  • Fragment props validation.
  • Type validation.
  • Key warning.

It's nice to be able to do some validation in the JSX/createElement because it has a more specific stack frame at the callsite. However, that's the case for every type of component and validation. That's the whole point of enableOwnerStacks. It's also not sufficient to do it in JSX/createElement so we also have validation in the renderers too. So this validation is really just an eager validation but also happens again later.

The problem with these is that we don't really know what types are valid until we get to the renderer. Additionally, by placing it in the isomorphic code it becomes harder to do deduping of warnings in a way that makes sense for that renderer. It also means we can't reuse logic for managing stacks etc.

Fragment props validation really should just be part of the renderer like any other component type. This also matters once we add Fragment refs and other fragment features. So I moved this into Fiber. However, since some Fragments don't have Fibers, I do the validation in ChildFiber instead of beginWork where it would normally happen.

For type validation we already do validation when rendering. By leaving it to the renderer we don't have to hard code an extra list. This list also varies by context. E.g. class components aren't allowed in RSC but client references are but we don't have an isomorphic way to identify client references because they're defined by the host config so the current logic is flawed anyway. I kept the early validation for now without the enableOwnerStacks since it does provide a nicer stack frame but with that flag on it'll be handled with nice stacks anyway. I normalized some of the errors to ensure tests pass.

For key validation it's the same principle. The mechanism for the heuristic is still the same - if it passes statically through a parent JSX/createElement call then it's considered validated. We already did print the error later from the renderer so this also disables the early log in the enableOwnerStacks flag.

I also added logging to Fizz so that key warnings can print in SSR logs.

Flight is a bit more complex. For elements that end up on the client we just pass the validated flag along to the client and let the client renderer print the error once rendered. For server components we log the error from Flight with the server component as the owner on the stack which will allow us to print the right stack for context. The factoring of this is a little tricky because we only want to warn if it's in an array parent but we want to log the error later to get the right debug info.

Fiber/Fizz has a similar factoring problem that causes us to create a fake Fiber for the owner which means the logs won't be associated with the right place in DevTools.

@react-sizebot
Copy link

react-sizebot commented May 16, 2024

Comparing: 2e540e2...43ceacd

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 = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 495.89 kB 495.90 kB = 88.79 kB 88.79 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 500.69 kB 500.70 kB = 89.47 kB 89.47 kB
facebook-www/ReactDOM-prod.classic.js = 593.45 kB 593.46 kB = 104.41 kB 104.41 kB
facebook-www/ReactDOM-prod.modern.js = 569.83 kB 569.84 kB = 100.80 kB 100.80 kB
oss-experimental/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
oss-stable-semver/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
oss-stable/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
facebook-react-native/react-is/cjs/ReactIs-dev.js +2.21% 6.37 kB 6.52 kB +3.66% 1.75 kB 1.81 kB
oss-experimental/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-stable-semver/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-stable/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-experimental/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-stable-semver/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-stable/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-experimental/react-server/cjs/react-server-flight.development.js +2.09% 119.35 kB 121.84 kB +2.38% 26.26 kB 26.88 kB
oss-experimental/react/cjs/react.development.js = 96.47 kB 78.25 kB = 26.28 kB 21.54 kB
oss-experimental/react/cjs/react.react-server.development.js = 76.27 kB 58.05 kB = 21.24 kB 16.56 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.react-server.development.js = 44.60 kB 27.55 kB = 13.31 kB 8.93 kB
oss-experimental/react/cjs/react-jsx-runtime.react-server.development.js = 44.60 kB 27.54 kB = 13.31 kB 8.93 kB
oss-experimental/react/cjs/react-jsx-runtime.development.js = 44.19 kB 27.16 kB = 13.18 kB 8.80 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.development.js = 42.96 kB 25.93 kB = 12.83 kB 8.45 kB
test_utils/ReactAllWarnings.js Deleted 64.39 kB 0.00 kB Deleted 16.06 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
oss-experimental/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
oss-stable-semver/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
oss-stable/react-dom/cjs/react-dom-test-utils.development.js +2.70% 2.37 kB 2.44 kB +0.77% 1.18 kB 1.19 kB
facebook-react-native/react-is/cjs/ReactIs-dev.js +2.21% 6.37 kB 6.52 kB +3.66% 1.75 kB 1.81 kB
oss-experimental/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-stable-semver/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-stable/react-is/cjs/react-is.development.js +2.18% 6.46 kB 6.60 kB +3.72% 1.77 kB 1.84 kB
oss-experimental/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-stable-semver/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-stable/react/cjs/react-compiler-runtime.development.js +2.15% 2.98 kB 3.04 kB +0.69% 1.44 kB 1.45 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.development.js +2.12% 3.02 kB 3.09 kB +0.81% 1.36 kB 1.37 kB
oss-experimental/react-server/cjs/react-server-flight.development.js +2.09% 119.35 kB 121.84 kB +2.38% 26.26 kB 26.88 kB
facebook-www/ReactIs-dev.modern.js +1.92% 7.33 kB 7.47 kB +3.59% 1.89 kB 1.96 kB
facebook-www/ReactIs-dev.classic.js +1.92% 7.33 kB 7.47 kB +3.64% 1.90 kB 1.96 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +1.45% 172.37 kB 174.87 kB +1.68% 37.91 kB 38.54 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +1.41% 177.30 kB 179.80 kB +1.61% 38.88 kB 39.51 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +1.40% 178.01 kB 180.50 kB +1.59% 39.12 kB 39.74 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +1.39% 179.22 kB 181.72 kB +1.62% 39.39 kB 40.02 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +1.39% 179.41 kB 181.90 kB +1.61% 39.46 kB 40.10 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +1.38% 180.24 kB 182.74 kB +1.62% 39.33 kB 39.97 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +1.38% 180.43 kB 182.92 kB +1.61% 39.40 kB 40.03 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +1.36% 182.89 kB 185.38 kB +1.59% 40.15 kB 40.78 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +1.36% 183.07 kB 185.56 kB +1.58% 40.22 kB 40.86 kB
oss-stable-semver/react-server/cjs/react-server-flight.development.js +1.25% 107.29 kB 108.63 kB +1.23% 23.65 kB 23.94 kB
oss-stable/react-server/cjs/react-server-flight.development.js +1.25% 107.29 kB 108.63 kB +1.23% 23.65 kB 23.94 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.96% 198.73 kB 200.63 kB +1.07% 46.86 kB 47.36 kB
oss-stable/react-server/cjs/react-server.development.js +0.96% 198.73 kB 200.63 kB +1.07% 46.86 kB 47.36 kB
oss-experimental/react-server/cjs/react-server.development.js +0.89% 214.07 kB 215.97 kB +0.97% 49.35 kB 49.83 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.84% 159.66 kB 161.00 kB +0.79% 35.09 kB 35.37 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.84% 159.66 kB 161.00 kB +0.79% 35.09 kB 35.37 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +0.81% 165.24 kB 166.58 kB +0.79% 36.29 kB 36.57 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +0.81% 165.24 kB 166.58 kB +0.79% 36.29 kB 36.57 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.81% 165.95 kB 167.29 kB +0.78% 36.52 kB 36.81 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.81% 165.95 kB 167.29 kB +0.78% 36.52 kB 36.81 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +0.80% 166.49 kB 167.83 kB +0.79% 36.56 kB 36.85 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +0.80% 166.49 kB 167.83 kB +0.79% 36.56 kB 36.85 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.80% 166.67 kB 168.01 kB +0.78% 36.63 kB 36.92 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.80% 166.67 kB 168.01 kB +0.78% 36.63 kB 36.92 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +0.80% 167.54 kB 168.87 kB +0.78% 36.52 kB 36.80 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +0.80% 167.54 kB 168.87 kB +0.78% 36.52 kB 36.80 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.80% 167.72 kB 169.06 kB +0.77% 36.58 kB 36.86 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.80% 167.72 kB 169.06 kB +0.77% 36.58 kB 36.86 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store-shim.native.development.js +0.79% 8.06 kB 8.12 kB +0.31% 3.18 kB 3.19 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-shim.native.development.js +0.79% 8.06 kB 8.12 kB +0.31% 3.18 kB 3.19 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-shim.native.development.js +0.79% 8.06 kB 8.12 kB +0.31% 3.18 kB 3.19 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.79% 170.18 kB 171.51 kB +0.76% 37.32 kB 37.60 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.79% 170.18 kB 171.51 kB +0.76% 37.32 kB 37.60 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.79% 170.36 kB 171.70 kB +0.75% 37.39 kB 37.67 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.79% 170.36 kB 171.70 kB +0.75% 37.39 kB 37.67 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store-shim.development.js +0.74% 8.64 kB 8.71 kB +0.27% 3.28 kB 3.29 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-shim.development.js +0.74% 8.64 kB 8.71 kB +0.27% 3.28 kB 3.29 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-shim.development.js +0.74% 8.64 kB 8.71 kB +0.27% 3.28 kB 3.29 kB
oss-experimental/react-cache/cjs/react-cache.development.js +0.72% 8.93 kB 8.99 kB +0.13% 2.99 kB 3.00 kB
oss-stable-semver/react-cache/cjs/react-cache.development.js +0.72% 8.93 kB 8.99 kB +0.13% 2.99 kB 3.00 kB
oss-stable/react-cache/cjs/react-cache.development.js +0.72% 8.93 kB 8.99 kB +0.13% 2.99 kB 3.00 kB
facebook-www/ReactTestRenderer-dev.modern.js +0.50% 846.34 kB 850.54 kB +0.55% 183.45 kB 184.46 kB
facebook-www/ReactTestRenderer-dev.classic.js +0.50% 846.35 kB 850.54 kB +0.55% 183.45 kB 184.46 kB
react-native/implementations/ReactFabric-dev.js +0.49% 860.68 kB 864.90 kB +0.56% 186.57 kB 187.62 kB
react-native/implementations/ReactNativeRenderer-dev.js +0.48% 873.87 kB 878.09 kB +0.54% 190.54 kB 191.57 kB
facebook-www/ReactCacheOld-dev.classic.js +0.47% 8.27 kB 8.31 kB +0.22% 2.73 kB 2.74 kB
facebook-www/ReactCacheOld-dev.modern.js +0.47% 8.27 kB 8.31 kB +0.22% 2.73 kB 2.74 kB
facebook-www/ReactART-dev.modern.js +0.47% 905.46 kB 909.68 kB +0.53% 193.28 kB 194.32 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.development.js +0.46% 410.57 kB 412.47 kB +0.52% 93.43 kB 93.92 kB
oss-stable/react-dom/cjs/react-dom-server.bun.development.js +0.46% 410.59 kB 412.49 kB +0.52% 93.45 kB 93.94 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.development.js +0.46% 807.24 kB 810.96 kB +0.57% 175.93 kB 176.94 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.development.js +0.46% 807.26 kB 810.99 kB +0.57% 175.96 kB 176.97 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.development.js +0.46% 808.52 kB 812.25 kB +0.55% 176.25 kB 177.23 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js +0.46% 415.70 kB 417.60 kB +0.53% 94.42 kB 94.92 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js +0.46% 415.72 kB 417.62 kB +0.53% 94.44 kB 94.94 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js +0.46% 416.53 kB 418.43 kB +0.51% 95.29 kB 95.78 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.46% 416.53 kB 418.43 kB +0.51% 95.29 kB 95.78 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js +0.46% 416.55 kB 418.45 kB +0.51% 95.31 kB 95.80 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.46% 416.55 kB 418.45 kB +0.51% 95.31 kB 95.80 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +0.46% 416.79 kB 418.69 kB +0.51% 95.57 kB 96.06 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +0.46% 416.82 kB 418.72 kB +0.51% 95.59 kB 96.08 kB
oss-stable-semver/react-art/cjs/react-art.development.js +0.46% 817.90 kB 821.63 kB +0.55% 177.55 kB 178.53 kB
oss-stable/react-art/cjs/react-art.development.js +0.46% 817.93 kB 821.65 kB +0.55% 177.57 kB 178.55 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.development.js +0.46% 417.38 kB 419.28 kB +0.52% 95.70 kB 96.19 kB
oss-stable/react-dom/cjs/react-dom-server.edge.development.js +0.46% 417.40 kB 419.30 kB +0.52% 95.72 kB 96.21 kB
oss-experimental/react-art/cjs/react-art.development.js +0.45% 825.75 kB 829.47 kB +0.54% 178.80 kB 179.77 kB
facebook-www/ReactART-dev.classic.js +0.45% 934.84 kB 939.06 kB +0.52% 199.19 kB 200.21 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +0.44% 423.34 kB 425.21 kB +0.48% 95.17 kB 95.63 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-dev.js +0.44% 852.12 kB 855.84 kB +0.52% 184.27 kB 185.23 kB
facebook-www/ReactDOMServer-dev.modern.js +0.44% 429.11 kB 430.99 kB +0.48% 96.56 kB 97.02 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +0.44% 435.96 kB 437.86 kB +0.48% 96.89 kB 97.36 kB
facebook-www/ReactDOMServer-dev.classic.js +0.43% 431.30 kB 433.18 kB +0.48% 97.03 kB 97.50 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +0.43% 441.92 kB 443.82 kB +0.47% 98.74 kB 99.21 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.43% 441.93 kB 443.83 kB +0.47% 98.74 kB 99.21 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +0.42% 449.21 kB 451.11 kB +0.47% 99.05 kB 99.52 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +0.42% 450.84 kB 452.74 kB +0.47% 99.90 kB 100.37 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +0.42% 451.42 kB 453.32 kB +0.47% 100.03 kB 100.50 kB
facebook-www/ReactReconciler-dev.modern.js +0.42% 1,009.19 kB 1,013.41 kB +0.48% 213.91 kB 214.94 kB
oss-experimental/react-dom/cjs/react-dom.react-server.development.js +0.41% 16.93 kB 17.00 kB +0.13% 3.74 kB 3.74 kB
oss-stable-semver/react-dom/cjs/react-dom.react-server.development.js +0.41% 16.93 kB 17.00 kB +0.13% 3.74 kB 3.74 kB
oss-stable/react-dom/cjs/react-dom.react-server.development.js +0.41% 16.93 kB 17.00 kB +0.13% 3.74 kB 3.74 kB
facebook-www/ReactReconciler-dev.classic.js +0.41% 1,039.12 kB 1,043.33 kB +0.46% 219.76 kB 220.78 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js +0.40% 920.76 kB 924.49 kB +0.49% 198.01 kB 198.99 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js +0.40% 920.78 kB 924.51 kB +0.49% 198.04 kB 199.01 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.40% 927.76 kB 931.48 kB +0.49% 199.25 kB 200.23 kB
react-native/implementations/ReactFabric-dev.fb.js +0.39% 952.91 kB 956.64 kB +0.48% 205.90 kB 206.87 kB
react-native/implementations/ReactNativeRenderer-dev.fb.js +0.39% 964.17 kB 967.90 kB +0.47% 209.29 kB 210.28 kB
oss-experimental/react-reconciler/cjs/react-reconciler-reflection.development.js +0.34% 19.09 kB 19.15 kB +0.11% 5.48 kB 5.48 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler-reflection.development.js +0.34% 19.09 kB 19.15 kB +0.11% 5.48 kB 5.48 kB
oss-stable/react-reconciler/cjs/react-reconciler-reflection.development.js +0.34% 19.09 kB 19.15 kB +0.11% 5.48 kB 5.48 kB
facebook-www/ReactDOM-dev.modern.js +0.30% 1,415.70 kB 1,419.91 kB +0.33% 309.96 kB 310.97 kB
facebook-www/ReactDOMTesting-dev.modern.js +0.29% 1,433.70 kB 1,437.91 kB +0.32% 314.31 kB 315.32 kB
facebook-www/ReactDOM-dev.classic.js +0.29% 1,456.18 kB 1,460.39 kB +0.33% 317.66 kB 318.69 kB
oss-stable-semver/react-dom/cjs/react-dom-client.development.js +0.29% 1,291.37 kB 1,295.09 kB +0.35% 287.07 kB 288.06 kB
oss-stable/react-dom/cjs/react-dom-client.development.js +0.29% 1,291.39 kB 1,295.12 kB +0.35% 287.09 kB 288.09 kB
oss-experimental/react-dom/cjs/react-dom-client.development.js +0.29% 1,298.36 kB 1,302.09 kB +0.35% 288.28 kB 289.28 kB
facebook-www/ReactDOMTesting-dev.classic.js +0.29% 1,474.18 kB 1,478.39 kB +0.32% 322.03 kB 323.06 kB
oss-stable-semver/react-dom/cjs/react-dom-profiling.development.js +0.28% 1,310.04 kB 1,313.77 kB +0.34% 290.45 kB 291.45 kB
oss-stable/react-dom/cjs/react-dom-profiling.development.js +0.28% 1,310.07 kB 1,313.79 kB +0.34% 290.48 kB 291.48 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.development.js +0.28% 1,316.48 kB 1,320.21 kB +0.35% 292.69 kB 293.72 kB
oss-experimental/react-dom/cjs/react-dom-profiling.development.js +0.28% 1,317.04 kB 1,320.77 kB +0.35% 291.67 kB 292.68 kB
oss-stable-semver/react-dom/cjs/react-dom.development.js +0.26% 24.72 kB 24.78 kB +0.14% 6.34 kB 6.34 kB
oss-stable/react-dom/cjs/react-dom.development.js +0.26% 24.74 kB 24.80 kB +0.20% 6.36 kB 6.37 kB
oss-experimental/react-dom/cjs/react-dom.development.js +0.26% 24.75 kB 24.81 kB +0.19% 6.36 kB 6.37 kB
oss-stable/react/cjs/react.development.js = 93.78 kB 93.56 kB = 25.73 kB 25.68 kB
oss-stable-semver/react/cjs/react.development.js = 93.76 kB 93.53 kB = 25.70 kB 25.66 kB
oss-stable/react/cjs/react.react-server.development.js = 69.47 kB 69.25 kB = 19.45 kB 19.43 kB
oss-stable-semver/react/cjs/react.react-server.development.js = 69.45 kB 69.23 kB = 19.43 kB 19.40 kB
facebook-react-native/react/cjs/React-dev.js = 104.71 kB 104.38 kB = 28.10 kB 28.05 kB
facebook-www/React-dev.classic.js = 116.81 kB 116.23 kB = 30.26 kB 30.16 kB
facebook-www/React-dev.modern.js = 116.32 kB 115.74 kB = 30.16 kB 30.06 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.react-server.development.js = 43.46 kB 42.96 kB = 13.11 kB 13.00 kB
oss-stable/react/cjs/react-jsx-dev-runtime.react-server.development.js = 43.46 kB 42.96 kB = 13.11 kB 13.00 kB
oss-stable-semver/react/cjs/react-jsx-runtime.react-server.development.js = 43.45 kB 42.96 kB = 13.11 kB 13.00 kB
oss-stable/react/cjs/react-jsx-runtime.react-server.development.js = 43.45 kB 42.96 kB = 13.11 kB 13.00 kB
facebook-react-native/react/cjs/JSXRuntime-dev.js = 43.85 kB 43.34 kB = 13.24 kB 13.13 kB
oss-stable-semver/react/cjs/react-jsx-runtime.development.js = 43.04 kB 42.54 kB = 12.99 kB 12.87 kB
oss-stable/react/cjs/react-jsx-runtime.development.js = 43.04 kB 42.54 kB = 12.99 kB 12.87 kB
facebook-react-native/react/cjs/JSXDEVRuntime-dev.js = 42.62 kB 42.11 kB = 12.88 kB 12.77 kB
oss-stable-semver/react/cjs/react-jsx-dev-runtime.development.js = 41.81 kB 41.31 kB = 12.64 kB 12.53 kB
oss-stable/react/cjs/react-jsx-dev-runtime.development.js = 41.81 kB 41.31 kB = 12.64 kB 12.53 kB
facebook-www/JSXDEVRuntime-dev.modern.js = 52.37 kB 51.62 kB = 14.71 kB 14.56 kB
facebook-www/JSXDEVRuntime-dev.classic.js = 52.34 kB 51.59 kB = 14.70 kB 14.56 kB
oss-experimental/react/cjs/react.development.js = 96.47 kB 78.25 kB = 26.28 kB 21.54 kB
oss-experimental/react/cjs/react.react-server.development.js = 76.27 kB 58.05 kB = 21.24 kB 16.56 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.react-server.development.js = 44.60 kB 27.55 kB = 13.31 kB 8.93 kB
oss-experimental/react/cjs/react-jsx-runtime.react-server.development.js = 44.60 kB 27.54 kB = 13.31 kB 8.93 kB
oss-experimental/react/cjs/react-jsx-runtime.development.js = 44.19 kB 27.16 kB = 13.18 kB 8.80 kB
oss-experimental/react/cjs/react-jsx-dev-runtime.development.js = 42.96 kB 25.93 kB = 12.83 kB 8.45 kB
test_utils/ReactAllWarnings.js Deleted 64.39 kB 0.00 kB Deleted 16.06 kB 0.00 kB

Generated by 🚫 dangerJS against 43ceacd

@sebmarkbage sebmarkbage force-pushed the elementstacks branch 2 times, most recently from c8e1421 to 168b54e Compare May 21, 2024 00:05
Copy link

vercel bot commented May 21, 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 May 23, 2024 4:42pm

'See https://react.dev/link/warning-keys for more information.',
gate(flags => flags.enableOwnerStacks)
? 'Each child in a list should have a unique "key" prop.' +
'\n\nCheck the top-level render call using <ParentClient>. ' +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got confused by this for a bit. I thought it suggested I should look inside ParentClient.

What do you think about

Suggested change
'\n\nCheck the top-level render call using <ParentClient>. ' +
'\n\nCheck the root.render() call using <ParentClient>. ' +

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to remove this whole contextual thing once we ship the owner stacks. It makes no sense and is kind of an artifact of trying to explain the actual cause which we don't have. I just didn't want to change it yet to keep parity in the incremental steps. Mainly because of all the tests. The current PR I'm working on I'm manually editing hundreds of tests. I'm actually doing a lot of incremental stuff I wouldn't do just because editing our tests is too much work.

[root]
▾ <Example>
▾ <Example>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we can get these back? It was part of the feature that you could jump to the element in devtools to find the offending element quickly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. So this is directly related to this TODO:

// TODO: Refactor the warnForMissingKey calls to happen after fiber creation
// so that we can get access to the fiber that will eventually be created.
// That way the log can show up associated with the right instance in DevTools.

Previously it was the "owner" of the parent which sometimes isn't related to where these elements are created at all. Now it's the actual child that should have the key specified on it but since that's a fake fiber it is not persistent in DevTools. We need to refactor ChildFiber to allow this warning to happen when we have access to the real fiber and it'll be better for context but I'm not planning on doing that yet because the rest of it is too much work already.

Comment on lines +168 to +169
// This only applies to the warning during construction.
// We do warn if it's actually rendered.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't this just a bug in the test? Component doesn't even accept children here. A more realistic case would probably implement Component to return <div>{children}</div> i.e. the test would end up rendering <div>{[[0, <Component />], [1, <Component />], [2, <Component />]]}</div> instead. Then we always warned because <Component /> was missing a key and i as well.

Feels like we should just add error assertions here and update the test description: #29192

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think it's still correct that it shouldn't error early in either case - which we have code to ensure in the JSX runtime. So this test is still valuable to assert that - at least until we remove that code all together. We already have tests for testing that maps warn when rendered.

Brandon-Isaac

This comment was marked as spam.

Eventually we should be able to delete describeUnknownElementTypeFrameInDEV
since it only has this one callsite.
We validate these in the renderers inherently anyway and the renderers
are the only ones that really knows what a valid type for that renderer is.

For example the Client Reference check isn't valid for all kinds of configs
in Flight. Also, a lot of the checks for valid elements are not valid in Flight
e.g. class components.

The only reason to do this validation early is because it provides a better
stack trace of the callsite for context. However, that's a general problem
and the whole point of enableOwnerStacks that it solves this for all kinds of
late errors.

For now I keep the early validation when enableOwnerStacks is off because
in that case we don't have a good stack (but to be fair we don't for any
other error neither). Once it is further rolled out we can delete these fully.
We have the same validation in the reconciler and with enableOwnerStacks
we'll have the ability to provide a stack trace with the callsite of the
element.

The way the validation works is that if something is passed through a valid
static slot, an element gets marked as valid. If it has been logged as errored
before, it gets logged as valid so that future logs don't happen.

This change logs for strictly fewer cases - specifically if an element doesn't
get rendered/mounted at all. I updated some tests to start rendering where it
didn't before.
When React.Children assigns an implicit key we should still warn when the
original element should've had a key since if was in an array.

This marks such elements as having failed validation even if they have a
key so that the error can later be logged in the renderer.
This is just additional for running warnings in the server logs in addition
to the ones that happen during hydration but it's good to have both for
parity and being able to fail SSR tests early.

This also demonstrates per request warning deduping so that they don't just
happen once when the server remains alive.
Elements that render Server Components are validated inside Flight.
Others pass the validated flag to the client.

Instead of running validation in certain cases (block list) we're marking
all the cases that don't need keys (allow list) which is tricky since
we need to cover all cases. This might lead to false warnings.
Key warnings are supposed to be associated with the child missing the key.
It was previously associated with the owner of the parent element which is
not really as useful but now it's unfortunately associated with a fake
fiber which disappears.
@sebmarkbage sebmarkbage merged commit 84239da into facebook:main May 23, 2024
40 checks passed
github-actions bot pushed a commit that referenced this pull request May 23, 2024
This is necessary to simplify the component stack handling to make way
for owner stacks. It also solves some hacks that we used to have but
don't quite make sense. It also solves the problem where things like key
warnings get silenced in RSC because they get deduped. It also surfaces
areas where we were missing key warnings to begin with.

Almost every type of warning is issued from the renderer. React Elements
are really not anything special themselves. They're just lazily invoked
functions and its really the renderer that determines there semantics.

We have three types of warnings that previously fired in
JSX/createElement:

- Fragment props validation.
- Type validation.
- Key warning.

It's nice to be able to do some validation in the JSX/createElement
because it has a more specific stack frame at the callsite. However,
that's the case for every type of component and validation. That's the
whole point of enableOwnerStacks. It's also not sufficient to do it in
JSX/createElement so we also have validation in the renderers too. So
this validation is really just an eager validation but also happens
again later.

The problem with these is that we don't really know what types are valid
until we get to the renderer. Additionally, by placing it in the
isomorphic code it becomes harder to do deduping of warnings in a way
that makes sense for that renderer. It also means we can't reuse logic
for managing stacks etc.

Fragment props validation really should just be part of the renderer
like any other component type. This also matters once we add Fragment
refs and other fragment features. So I moved this into Fiber. However,
since some Fragments don't have Fibers, I do the validation in
ChildFiber instead of beginWork where it would normally happen.

For `type` validation we already do validation when rendering. By
leaving it to the renderer we don't have to hard code an extra list.
This list also varies by context. E.g. class components aren't allowed
in RSC but client references are but we don't have an isomorphic way to
identify client references because they're defined by the host config so
the current logic is flawed anyway. I kept the early validation for now
without the `enableOwnerStacks` since it does provide a nicer stack
frame but with that flag on it'll be handled with nice stacks anyway. I
normalized some of the errors to ensure tests pass.

For `key` validation it's the same principle. The mechanism for the
heuristic is still the same - if it passes statically through a parent
JSX/createElement call then it's considered validated. We already did
print the error later from the renderer so this also disables the early
log in the `enableOwnerStacks` flag.

I also added logging to Fizz so that key warnings can print in SSR logs.

Flight is a bit more complex. For elements that end up on the client we
just pass the `validated` flag along to the client and let the client
renderer print the error once rendered. For server components we log the
error from Flight with the server component as the owner on the stack
which will allow us to print the right stack for context. The factoring
of this is a little tricky because we only want to warn if it's in an
array parent but we want to log the error later to get the right debug
info.

Fiber/Fizz has a similar factoring problem that causes us to create a
fake Fiber for the owner which means the logs won't be associated with
the right place in DevTools.

DiffTrain build for commit 84239da.
github-actions bot pushed a commit that referenced this pull request May 23, 2024
This is necessary to simplify the component stack handling to make way
for owner stacks. It also solves some hacks that we used to have but
don't quite make sense. It also solves the problem where things like key
warnings get silenced in RSC because they get deduped. It also surfaces
areas where we were missing key warnings to begin with.

Almost every type of warning is issued from the renderer. React Elements
are really not anything special themselves. They're just lazily invoked
functions and its really the renderer that determines there semantics.

We have three types of warnings that previously fired in
JSX/createElement:

- Fragment props validation.
- Type validation.
- Key warning.

It's nice to be able to do some validation in the JSX/createElement
because it has a more specific stack frame at the callsite. However,
that's the case for every type of component and validation. That's the
whole point of enableOwnerStacks. It's also not sufficient to do it in
JSX/createElement so we also have validation in the renderers too. So
this validation is really just an eager validation but also happens
again later.

The problem with these is that we don't really know what types are valid
until we get to the renderer. Additionally, by placing it in the
isomorphic code it becomes harder to do deduping of warnings in a way
that makes sense for that renderer. It also means we can't reuse logic
for managing stacks etc.

Fragment props validation really should just be part of the renderer
like any other component type. This also matters once we add Fragment
refs and other fragment features. So I moved this into Fiber. However,
since some Fragments don't have Fibers, I do the validation in
ChildFiber instead of beginWork where it would normally happen.

For `type` validation we already do validation when rendering. By
leaving it to the renderer we don't have to hard code an extra list.
This list also varies by context. E.g. class components aren't allowed
in RSC but client references are but we don't have an isomorphic way to
identify client references because they're defined by the host config so
the current logic is flawed anyway. I kept the early validation for now
without the `enableOwnerStacks` since it does provide a nicer stack
frame but with that flag on it'll be handled with nice stacks anyway. I
normalized some of the errors to ensure tests pass.

For `key` validation it's the same principle. The mechanism for the
heuristic is still the same - if it passes statically through a parent
JSX/createElement call then it's considered validated. We already did
print the error later from the renderer so this also disables the early
log in the `enableOwnerStacks` flag.

I also added logging to Fizz so that key warnings can print in SSR logs.

Flight is a bit more complex. For elements that end up on the client we
just pass the `validated` flag along to the client and let the client
renderer print the error once rendered. For server components we log the
error from Flight with the server component as the owner on the stack
which will allow us to print the right stack for context. The factoring
of this is a little tricky because we only want to warn if it's in an
array parent but we want to log the error later to get the right debug
info.

Fiber/Fizz has a similar factoring problem that causes us to create a
fake Fiber for the owner which means the logs won't be associated with
the right place in DevTools.

DiffTrain build for [84239da](84239da)
yungsters added a commit that referenced this pull request May 30, 2024
## Summary

#29088 introduced a regression
triggering this warning when rendering flattened positional children:

> Each child in a list should have a unique "key" prop.

The specific scenario that triggers this is when rendering multiple
positional children (which do not require unique `key` props) after
flattening them with one of the `React.Children` utilities (e.g.
`React.Children.toArray`).

The refactored logic in `React.Children` incorrectly drops the
`element._store.validated` property in `__DEV__`. This diff fixes the
bug and introduces a unit test to prevent future regressions.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```
github-actions bot pushed a commit that referenced this pull request May 30, 2024
## Summary

#29088 introduced a regression
triggering this warning when rendering flattened positional children:

> Each child in a list should have a unique "key" prop.

The specific scenario that triggers this is when rendering multiple
positional children (which do not require unique `key` props) after
flattening them with one of the `React.Children` utilities (e.g.
`React.Children.toArray`).

The refactored logic in `React.Children` incorrectly drops the
`element._store.validated` property in `__DEV__`. This diff fixes the
bug and introduces a unit test to prevent future regressions.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```

DiffTrain build for commit 72644ef.
github-actions bot pushed a commit that referenced this pull request May 30, 2024
## Summary

#29088 introduced a regression
triggering this warning when rendering flattened positional children:

> Each child in a list should have a unique "key" prop.

The specific scenario that triggers this is when rendering multiple
positional children (which do not require unique `key` props) after
flattening them with one of the `React.Children` utilities (e.g.
`React.Children.toArray`).

The refactored logic in `React.Children` incorrectly drops the
`element._store.validated` property in `__DEV__`. This diff fixes the
bug and introduces a unit test to prevent future regressions.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```

DiffTrain build for [72644ef](72644ef)
yungsters added a commit that referenced this pull request May 31, 2024
## Summary

In #29088, the validation logic
for `React.Children` inspected whether `mappedChild` — the return value
of the map callback — has a valid `key`. However, this deviates from
existing behavior which only warns if the original `child` is missing a
required `key`.

This fixes false positive `key` validation warnings when using
`React.Children`, by validating the original `child` instead of
`mappedChild`.

This is a more general fix that expands upon my previous fix in
#29662.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```
github-actions bot pushed a commit that referenced this pull request May 31, 2024
## Summary

In #29088, the validation logic
for `React.Children` inspected whether `mappedChild` — the return value
of the map callback — has a valid `key`. However, this deviates from
existing behavior which only warns if the original `child` is missing a
required `key`.

This fixes false positive `key` validation warnings when using
`React.Children`, by validating the original `child` instead of
`mappedChild`.

This is a more general fix that expands upon my previous fix in
#29662.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```

DiffTrain build for commit 8fd963a.
github-actions bot pushed a commit that referenced this pull request May 31, 2024
## Summary

In #29088, the validation logic
for `React.Children` inspected whether `mappedChild` — the return value
of the map callback — has a valid `key`. However, this deviates from
existing behavior which only warns if the original `child` is missing a
required `key`.

This fixes false positive `key` validation warnings when using
`React.Children`, by validating the original `child` instead of
`mappedChild`.

This is a more general fix that expands upon my previous fix in
#29662.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```

DiffTrain build for [8fd963a](8fd963a)
hoxyq added a commit that referenced this pull request Jun 6, 2024
#29777)

## Summary

The test started to fail after
#29088.

Fork the test and the expected store state for:
- React 18.x, to represent the previous behavior
- React >= 19, to represent the current RDT behavior, where error can't
be connected to the fiber, because it was not yet mounted and shared
with DevTools.

Ideally, DevTools should start keeping track of such fibers, but also
distinguish them from some that haven't mounted due to Suspense or error
boundaries.
github-actions bot pushed a commit that referenced this pull request Jun 6, 2024
#29777)

## Summary

The test started to fail after
#29088.

Fork the test and the expected store state for:
- React 18.x, to represent the previous behavior
- React >= 19, to represent the current RDT behavior, where error can't
be connected to the fiber, because it was not yet mounted and shared
with DevTools.

Ideally, DevTools should start keeping track of such fibers, but also
distinguish them from some that haven't mounted due to Suspense or error
boundaries.

DiffTrain build for commit fe5ce4e.
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))
sebmarkbage added a commit that referenced this pull request Jun 27, 2024
This is all behind the `enableOwnerStacks` flag.

This is a follow up to #29088. In that I moved type validation into the
renderer since that's the one that knows what types are allowed.
However, I only removed it from `React.createElement` and not the JSX
which was an oversight.

However, I also noticed that for invalid types we don't have the right
stack trace for throws because we're not yet inside the JSX element that
itself is invalid. We should use its stack for the stack trace. That's
the reason it's enough to just use the throw now because we can get a
good stack trace from the owner stack. This is fixed by creating a fake
Throw Fiber that gets assigned the right stack.

Additionally, I noticed that for certain invalid types like the most
common one `undefined` we error in Flight so a missing import in RSC
leads to a generic error. Instead of erroring on the Flight side we
should just let anything that's not a Server Component through to the
client and then let the Client renderer determine whether it's a valid
type or not. Since we now have owner stacks through the server too, this
will still be able to provide a good stack trace on the client that
points to the server in that case.

<img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM"
src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4">

To get the best stack you have to expand the little icon and the regular
stack is noisy [due to this Chrome
bug](https://issues.chromium.org/issues/345248263) which makes it a
little harder to find but once that's fixed it might be easier.
github-actions bot pushed a commit that referenced this pull request Jun 27, 2024
This is all behind the `enableOwnerStacks` flag.

This is a follow up to #29088. In that I moved type validation into the
renderer since that's the one that knows what types are allowed.
However, I only removed it from `React.createElement` and not the JSX
which was an oversight.

However, I also noticed that for invalid types we don't have the right
stack trace for throws because we're not yet inside the JSX element that
itself is invalid. We should use its stack for the stack trace. That's
the reason it's enough to just use the throw now because we can get a
good stack trace from the owner stack. This is fixed by creating a fake
Throw Fiber that gets assigned the right stack.

Additionally, I noticed that for certain invalid types like the most
common one `undefined` we error in Flight so a missing import in RSC
leads to a generic error. Instead of erroring on the Flight side we
should just let anything that's not a Server Component through to the
client and then let the Client renderer determine whether it's a valid
type or not. Since we now have owner stacks through the server too, this
will still be able to provide a good stack trace on the client that
points to the server in that case.

<img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM"
src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4">

To get the best stack you have to expand the little icon and the regular
stack is noisy [due to this Chrome
bug](https://issues.chromium.org/issues/345248263) which makes it a
little harder to find but once that's fixed it might be easier.

DiffTrain build for commit e02baf6.
github-actions bot pushed a commit that referenced this pull request Jun 27, 2024
This is all behind the `enableOwnerStacks` flag.

This is a follow up to #29088. In that I moved type validation into the
renderer since that's the one that knows what types are allowed.
However, I only removed it from `React.createElement` and not the JSX
which was an oversight.

However, I also noticed that for invalid types we don't have the right
stack trace for throws because we're not yet inside the JSX element that
itself is invalid. We should use its stack for the stack trace. That's
the reason it's enough to just use the throw now because we can get a
good stack trace from the owner stack. This is fixed by creating a fake
Throw Fiber that gets assigned the right stack.

Additionally, I noticed that for certain invalid types like the most
common one `undefined` we error in Flight so a missing import in RSC
leads to a generic error. Instead of erroring on the Flight side we
should just let anything that's not a Server Component through to the
client and then let the Client renderer determine whether it's a valid
type or not. Since we now have owner stacks through the server too, this
will still be able to provide a good stack trace on the client that
points to the server in that case.

<img width="571" alt="Screenshot 2024-06-25 at 6 46 35 PM"
src="https://github.com/facebook/react/assets/63648/6812c24f-e274-4e09-b4de-21deda9ea1d4">

To get the best stack you have to expand the little icon and the regular
stack is noisy [due to this Chrome
bug](https://issues.chromium.org/issues/345248263) which makes it a
little harder to find but once that's fixed it might be easier.

DiffTrain build for [e02baf6](e02baf6)
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.

6 participants