[React 19] custom element property vs. attribute behavior deviates from RFC discussion and proposal doc

[cjpillsbury]

Summary

Hey folks! Not sure if the decisions here changed and I missed it, but it looks like the current React 19 beta impl for react props -> web component props/attrs has changed from what was originally discussed by @josepharhar. The tl;dr:

Unlike Preact, "complex values" were originally going to only be assigned to properties (if/when they exist). If the property does not exist, no value would be assigned. Instead, only functions and Symbols are omitted from setting as attributes.

Not only does this result in likely undesired behavior (e.g. anattr="[object Object]"), it can also result in runtime errors that are potentially difficult to debug, e.g.:

<my-wc iThoughtThisWasAProp={[aSymbol]}><my-wc> // This will crash

It looks like the originally planned logic could be introduced in this general vicinity. Not sure if this is a bug, a feature request, or just a discussion as to why the change in plans, but just wanted to surface for visibility, just in case. Also, happy to cobble together a codesandbox or similar if that would be helpful!

[josepharhar]

It's been a while, and I don't remember what the thinking as around Symbols. A codesandbox would be helpful!

[cjpillsbury]

The primary issue isn't Symbol per say, though an array with a Symbol value in it is particularly eggregious, since it causes an app crash. The tl;dr - in the original discussion (at least as I understood it, unless I missed something), complex values (e.g. typeof val === 'object', maybe with exceptions for null, though plausibly unnecessary) would never be set as attributes. That makes sense for consistency between (eventual) SSR/partial hydration, but also from a usefulness perspective for well-designed Web Components.

Here's a stackblitz example with a bunch of comments talking through some of the permutations:
https://stackblitz.com/edit/vitejs-vite-yrmfxm?file=src%2FApp.jsx

Happy to discuss more. It does look like inserting this logic should be narrow (per my prior comment+link), though I 💯 could be missing some larger scope concerns/considerations.

[josepharhar]

Thanks for the example with comments. It seems like all of the problematic cases are indeed when we aren't running on the client/browser or when the custom element hasn't been upgraded yet, and you currently have to work around it by doing things like situation number 1 with example code in your comment here.

This is still a limitation of the support for setting values as object properties instead of attributes in React. I don't know if the web components ecosystem will mature enough for step 3 in my doc, but I suppose that step 2 is something that is worth working on - although I'm not familiar enough with Suspense boundaries or server rendering to figure out how to prevent rendering custom elements until they are upgraded.

I can say that one of the pieces to prevent custom elements from being rendered until they are upgraded would be to use whenDefined() within React for every custom element that is encountered. I'm also not sure how difficult this would be to implement within React's architecture.

I'm not sure I can prioritize diving into this over my other work, so help would be appreciated.

[cjpillsbury]

Cool I think I read

I propose that we render nothing for objects, arrays, and functions. I’ve heard that people are using JSON.stringify already if they really want to stringify their objects.

as part of step 1, given

Implement Preact-like support for client side rendering, but probably with some tweaks to improve the behavior.

and also since it seems like an improvement regardless of any async and/or SSR support. Not a callout, just highlighting where the misunderstanding occurred.

I'm happy to take a stab at a PR (also likely within the bounds of my other obligations). Might be able to get something going this weekend 🤞, with the tl;dr being - refactor core impl so React only ever sets properties if the value is an array or an object (this is already true for functions).

Also, tangentially related: I'm currently doing a blog post writeup on the status of all this crud and noting a few other places for potential improvements or at least discussion. Let me know if you have any gut reactions to these, think they need more broad discussion, suggestions on opening new issues, etc. etc. Here are a couple:

1. sorting the react props before running the under the hood logic for event handler vs. prop vs. attribute setting so that the "event handler candidates" are always applied first
   i. Motivation: many react developers aren't used to having to reason about order in their JSX, but at least some event handlers may be dispatched as a result of other properties/attributes getting set or simply based on initialization crud. This will at least reduce the cognitive load for React devs
2. Consider changing the order of checks for "event handler" vs. "there is a property with a corresponding name"
   i. Motivation: There are edge cases where a property reasonably exists that isn't for events (e.g. onto onboard, etc.). The current workaround if these accept functions (that doesn't involve refs + hooks) is for Web Component authors to add a second accessor with a different name for the same purpose. There are also cases where, like "built in" HTML elements, there are corresponding properties for event handlers e.g. an onclick property. Assuming Web Component authors are implementing these correctly, these should work fine for the event use case.

Thanks for the convo/updates. I'll be sure to link relevant issues to any PRs opened. I'll also try to peel away some time to see what kind of concerns there are on the Next.JS side of the fence.

[cjpillsbury]

I can say that one of the pieces to prevent custom elements from being rendered until they are upgraded would be to use whenDefined() within React for every custom element that is encountered. I'm also not sure how difficult this would be to implement within React's architecture.

I do think this should be treated as a separate effort. Thus far I surprisingly haven't encountered this issue with basic smoke testing (surprisingly, since I'm presuming from your discussion that there's been nothing implemented to guarantee this)