-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Web Component custom attributes missing when using Preact #2860
Comments
I know what the issue is now and how to fix it (both my code and Preact) so I'm updating my Web Component and the main link (not the CodePen one) will likely be fixed later today. It seems an edge case issue and since Web Components are not really very popular compared to using Preact/React Components so I think it's something that could be just be documented in case it's not going to be supported (might add to much code or slow down performance too much). Condition that causes the error // Only getter is defined
get url() { return this.getAttribute('url'); } Fix for Web Component Developers // Define both getter and setter
get url() { return this.getAttribute('url'); }
set url(newValue) { return this.setAttribute('url', newValue); } How this works in Preact https://github.com/preactjs/preact/blob/master/src/diff/props.js#L107 // At lines 116 and 117 in the current master branch the following condition evaluates
// `true` if only a getter is defined so the result is element property gets set
// and not the HTML Attribute:
!isSvg &&
name in dom
) {
dom[name] = value == null ? '' : value; Possible Fix // Ideal Case - Adding only one line
!isSvg &&
name in dom &&
Object.getOwnPropertyDescriptor(dom.__proto__, name).set !== undefined
) {
dom[name] = value == null ? '' : value;
// Worst Case - multiple files would need to be updated extra code needed, quick pseudo example:
// [diff/index.js] - new `isWebComponent` where `isSvg` is called
isSvg = newVNode.type === 'svg' || isSvg;
isWebComponent = newVNode.type.includes('-') || (newProps && typeof newProps.is === 'string')
// Then in [/diff/props.js] `isWebComponent` would be passed and more logic used
!isSvg &&
((!isWebComponent && name in dom)
|| (isWebComponent && name in dom && Object.getOwnPropertyDescriptor(dom.__proto__, name).set !== undefined))
) {
dom[name] = value == null ? '' : value; While the ideal case listed above is small code change I would expect it to require a lot of code for basic unit testing and I'm not certain it wouldn't affect regular elements (quick test I did showed it would not in Chrome) but to be certain a change like this would require significant testing and manually with many different browsers on different devices (example widely used older Chromium Browsers or Safari which have partial Web Component Support). Also as to why this works with React and not Preact the related React Code is here: It ends up calling this which is significant in size: If Preact maintainers would be willing to considering handling getter only properties I would be happy to help out in making the change, unit tests, or even help testing with different mobile devices; granted doesn't seem like this will be on anyone's priority list. |
Final follow up unless anyone respond with questions. // This earlier example above:
!isSvg &&
name in dom &&
Object.getOwnPropertyDescriptor(dom.__proto__, name).set !== undefined
) {
dom[name] = value == null ? '' : value;
// Would need to be more like this (however optimized to avoid multiple calls using `Object.get...`)
!isSvg &&
name in dom &&
Object.getOwnPropertyDescriptor(Object.getPrototypeOf(dom), name).set !== undefined &&
Object.getOwnPropertyDescriptor(Object.getPrototypeOf(dom), name).writable !== false
) {
dom[name] = value == null ? '' : value; I see that the release builds of Preact do not include |
Going to close out this issue I opened. I think it's a rare case and not worth adding extra code to Preact for. Vue 3 has the same behavior as well when using Web Components. If anyone is reading this with the same issue just make sure you define both a getter and setter when the HTML attribute and DOM Property have the same name for an Custom Element. |
Good find! I'm curious if we should switch and add Regarding the |
Thanks @marvinhagemeister Yeah I agree with I did a quick check of several of the most widely used Libraries and found the following list are all using
Here is a comment from the latest version of jQuery near the top: https://code.jquery.com/jquery-3.5.1.js // Edge <= 12 - 13+, Firefox <=18 - 45+, IE 10 - 11, Safari 5.1 - 9+, iOS 6 - 9.1
// throw exceptions when non-strict code (e.g., ASP.NET 4.5) accesses strict mode
// arguments.callee.caller (trac-13335). But as of jQuery 3.0 (2016), strict mode should be common
// enough that all such attempts are guarded in a try block.
"use strict"; You are understanding issue correctly in that DOM properties are not reflected back to the attributes. Here are a few links in case you want to take a glance:
Snippet from the first link: export class MyCustomElement extends HTMLElement {
...
get disabled() {
return this.hasAttribute('disabled');
}
set disabled(val) {
if(val) {
this.setAttribute('disabled', val);
} else {
this.removeAttribute('disabled');
}
}
}
static get observedAttributes() {
return ['disabled'];
}
attributeChangedCallback(name, oldValue, newValue) {
this.disabled = newValue;
} As you can see it's a lot of extra code to make the attribute always match the value of the property. I've been experimenting with Web Components recently in some production apps and found that all the extra code results in myself (and I would assume other developers) not adding Related to how Vue 3 handles this it provides a warning (shown in screenshow below) if using the uncompressed build, however the error doesn't show up when using the compressed production build. Uncompressed build: https://cdn.jsdelivr.net/npm/[email protected]/dist/vue.global.js To see how it would be have with Preact I did some testing with and without When including https://unpkg.com/[email protected]/debug/dist/debug.umd.js this error shows if I've researched performance in the past related to
However I would be hesitant to add it to Preact 10 because it might break code for developers who use Preact. For Preact 11 I think it would be a good addition to add. Although now as I write this, I think it wouldn't have any impact on 99%+ of apps (just guessing), but at the very least it would need to be on a minor release (e.g.: |
Thank you for finding. I just face same, (or similar), problem in NextJS (12.0.4-canary.5 currently). I use custom element (build in VueJS 2) and for production I replace React with Preact. When I load page first time, element render without problems. When I use React, problem go away. Any advice how to solve this issue? Thanks. |
@havran Here is a SPA that uses Preact with Web Components. On the "Data Example" page it uses a web component with single word attribute Demo Source HTML Source JSX |
@ConradSollitt Hi, thanks for answer. I try recreate same bug, with simple web component and basic NextJS app, which I create, but without success. Simple component work. Seems like problem is related with component which we use, because when I try use same component, then I get same problem. When I found some more, I add this info here. Thanks again. |
Preact's behavior of not rendering attributes to custom elements creates issues in our isomorphic environment. We pre-render web components by hydrating them on the server-side. In a nutshell, web components are rendered with light DOM and scoped CSS instead of Shadow DOM. This way we can guarantee that our web components are displayed on the client-side before the application is available. The server-side hydration of web components operates on a DOM that was created using preact. Because of the missing attributes on custom elements, the hydration is missing meaningful instructions that otherwise would be available when added as attributes. It seems that the React team stumbled upon this caveat while working on web component support. It would be great if this issue could be reopened to continue the discussion to set attributes rather than assigning property values for web components when rendered with Preact. |
Having explicit control over setting JS properties vs DOM attributes on elements would alleviate issues like this one in the OP because a user could elect to set an attribute instead of a JS property. I've made a proposal to add a new test to And here's the issue I made to track the idea for Preact: |
Greetings, I've found a possible issue when using Preact with Web Components (Custom Elements) where custom HTML attributes are not being assigned to the element when rendered by Preact. Originally I had created this as a React demo to make sure the Components work with React so the code is almost identical except the Preact version loads Preact instead of React of course.
Alternatively perhaps the error is with the Web Component and React is just handling it but so far in my research everything with the Web Component seems good and it works well with regular HTML.
Reproduction
https://codepen.io/conrad-sollitt/pen/JjREjQg
Preact Version for the Demo
Original Demo (Preact)
https://www.dataformsjs.com/examples/web-components-with-preact.htm
Based on React Version that Works
Both files share the same JSX
https://www.dataformsjs.com/examples/web-components-with-react.htm
Steps to reproduce
Data
Page from the top nav.Expected Behavior
url
custom HTML attribute is included.Additional Notes
HTML Source for the Demo
https://github.com/dataformsjs/dataformsjs/blob/master/examples/web-components-with-preact.htm
JSX Source for the Demo
https://github.com/dataformsjs/dataformsjs/blob/master/examples/web-components-with-react.jsx
Source for the
<json-data>
Web Componenthttps://github.com/dataformsjs/dataformsjs/blob/master/js/web-components/json-data.js
The minified version is simply minified using Terser and not compiled down to ES5 or earlier versions of JS
Source for working
<markdown-content>
Web Componenthttps://github.com/dataformsjs/dataformsjs/blob/master/js/web-components/markdown-content.js
Intresting the Markdown page is working and it too uses a
url
attributeBoth files have a similar layout and:
I'll still do some more work to see if I can track down the issue myself.
The text was updated successfully, but these errors were encountered: