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
[React 19] custom element property vs. attribute behavior deviates from RFC discussion and proposal doc #29037
Comments
It's been a while, and I don't remember what the thinking as around Symbols. A codesandbox would be helpful! |
The primary issue isn't Here's a stackblitz example with a bunch of comments talking through some of the permutations: 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. |
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. |
Cool I think I read
as part of step 1, given
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:
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. |
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) |
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.: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!
The text was updated successfully, but these errors were encountered: