Closed ConradSollitt closed 3 years ago
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: https://github.com/facebook/react/blob/master/packages/react-dom/src/client/DOMPropertyOperations.js#L136
It ends up calling this which is significant in size: https://github.com/facebook/react/blob/master/packages/react-dom/src/shared/DOMProperty.js#L207
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 "use strict";
so perhaps the error I originally described would have been caught if I used webpack or node tools. If Preact did include "use strict";
then this line dom[name] = value == null ? '' : value;
would have thrown an error from Preact.
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 "use strict";
at the top of our code that's packaged for npm. That's a difficult issue to track down, much respect to ya! If I'm understanding it correctly, the problem is that the DOM properties are not reflected back to the attributes, right?
Regarding the getOwnPropertyDescriptor
: I'm worried that this is expensive. Don't have any current numbers to back that up, something worth to check, but I'm willing to bet that there must be a better way.
Thanks @marvinhagemeister
Yeah I agree with getOwnPropertyDescriptor
which is why I closed out this issue but after reading our comment on "use strict";
I think it's definitely worth looking into and considering.
I did a quick check of several of the most widely used Libraries and found the following list are all using use strict
:
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 setter
functions or other features unless it's really needed. In my original example it made sense to originally have a getter but I didn't realize the need for a setter at first so I didn't add one.
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/vue@3.0.4/dist/vue.global.js
To see how it would be have with Preact I did some testing with and without use strict
. I took the minimized production version and add it at the top.
When including https://unpkg.com/preact@10.5.7/debug/dist/debug.umd.js this error shows if use strict
is included but if the debug file is not included then no error shows up (which is a similar result to Vue 3).
I've researched performance in the past related to use strict
and I know for some code it can increase performance (not sure if enough for an end user to notice though). Quick google result for some relevant links from Stack Overflow:
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.: 10.6.0
) rather than a patch release (10.5.8
if published).
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 navigate from page, which contain element, to different page and then navigate back, element lost some attributes - I realize only 'single word' attributes are missing.
When I use React, problem go away.
Any advice how to solve this issue? Thanks.
@havran Can you provide an example or some code? Sounds like it might be an issue with NextJS.
Here is a SPA that uses Preact with Web Components. On the "Data Example" page it uses a web component with single word attribute <json-data url="{url}">
. When you click off and on the page it renders correctly. For the SPA functionality React Router is used.
Demo https://dataformsjs.com/examples/web-components-with-preact.htm#/
Source HTML https://github.com/dataformsjs/dataformsjs/blob/master/examples/web-components-with-preact.htm
Source JSX The JSX is shared by both React and Preact Demos https://github.com/dataformsjs/dataformsjs/blob/master/examples/web-components-with-react.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 custom-elements-everywhere
to cover this, which would ensure that complying frameworks allow users to pick whether they set properties vs attributes:
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 Component https://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 JSSource for working
<markdown-content>
Web Component https://github.com/dataformsjs/dataformsjs/blob/master/js/web-components/markdown-content.js Intresting the Markdown page is working and it too uses aurl
attribute Both files have a similar layout and:I'll still do some more work to see if I can track down the issue myself.