Closed DCsunset closed 1 day ago
This is running afoul of our props vs attribute detection, as they've unfortunately defined a rates
getter & setter which means Preact assumes they want this set as a property, rather than an attribute.
I see. Is there any workaround for this? I tried capitalizing it but it still doesn't work.
Refs, for example:
<MediaPlaybackRateButton ref={(d) => d.setAttribute('rates', '1 2 3')} />
Just a tiny note:
unfortunately defined a getter
having a property is what you'd expect from a web component, is it not? IDL attributes are supposed to be reflections of their content counterpart; preacts current behaviour forces CEs to either:
having a property is what you'd expect from a web component, is it not?
Potentially, but it's the combination of defining a getter/setter yet expecting attributes to be set. Preact assumes that if you've set a getter/setter, you'd expect to Preact/your framework use those rather than switch to attributes.
change the content attribute when the idl part changes
Potentially, yes. Being a JS framework, I don't think this is problematic; properties are the native interface for interacting with DOM elements. Most properties don't need to be reflected back to attributes, and a few shouldn't. That's not to say the use case is invalid, but that, IMO, properties should be the interface of choice and "the intended order" so to speak.
you'd expect your framework to use those
But I don't expect any framework, that's why I'm writing a CE ;) anyway, this discussion should probably be done in the mentioned PR, if at all.
And about the order: :woman_shrugging: possibly. I just find it odd that since the content attribute is there first and the CE is actively upgraded to read the content attribute into its IDL attribute that from there on out the IDL attribute should take charge. In my case I've done just that and it works as expected. Looking through webcomponents.org (specifically githubs own CEs) many also do just that.
I just find it odd that since the content attribute is there first
Depends on your usage. If you're registering a Preact component as a custom element, and are already using it in your HTML doc with defined attributes, sure, I agree. This issue shows a different usage though, with a Preact wrapper around a custom element -- it doesn't exist until Preact renders it. I don't think there is a right or wrong here, it's always going to be a guess.
I'm not sure if there's any consensus/strong opinions one way or the other but we'll update the PR when/if there is.
Custom Elements that define public properties are supposed to reflect them to attributes, since that's how all of the built-in elements work. Sure, a CE can just not do that, but the overwhelming majority of them do. It's part of what makes a Custom Element an Element, rather than just a framework-agnostic blob of JS mounted into the DOM.
Things look/feel broken when elements don't do this:
const el = document.createElement('foo-whatever')
el.setAttribute('rates', '1')
el.getAttribute('rates') // '1'
console.log(el.rates) // '1'
el.rates = '2'
console.log(el.rates) // '2'
console.log(el.getAttribute('rates')) // '1' ??
Closing this as after further research I align with @developit's explanation here.
@rschristian Don't wanna "necro" this, but while I can follow your explanation I don't think people that use frameworks view their written JSX expressions as an alternative way to create JS instances and set values on them. They see "html in JS and somehow there's functions now". That's a POV that only thinks about the content attributes in my experience. Which is why trying to explicitly control e.g. a details elements open state can be really confusing because the details element does exactly what's being suggested here.
I think both react and preact need to think long and hard whether they still want to take the "JS First" Position when it comes to actual elements.
Describe the bug When using Preact with react components in
media-chrome
package, the props are not passed to the component correctly. For example, the rates forMediaPlaybackButton
couldn't be set correctly. (it works correctly when using React)To Reproduce
StackBlitz: https://stackblitz.com/edit/create-preact-starter-l7wbnq?file=src%2Findex.jsx
Steps to reproduce the behavior:
Expected behavior What should have happened when following the steps above?
rates = "1 2 3"
(docs: https://www.media-chrome.org/docs/en/components/media-playback-rate-button)