react icon indicating copy to clipboard operation
react copied to clipboard

Move createElement/JSX Warnings into the Renderer

Open sebmarkbage opened this issue 1 month ago • 2 comments

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.

sebmarkbage avatar May 16 '24 02:05 sebmarkbage