Skip to content

Conversation

VelizarD
Copy link
Contributor

@VelizarD VelizarD commented Sep 15, 2025

With #108 the register method now requires the propNames array to contain the prop names in camelcase which is incorrect:
image

With my changes only the required kebab case props taken from the component are allowed to be passed:
image

@rschristian
Copy link
Member

the register method now requires the propNames array to contain the prop names in camelcase which is incorrect:

Er, no? It requires observedAttributes match your component props, whatever naming convention you use.

With my changes only the required kebab case props taken from the component are allowed to be passed:

This absolutely should not be done. HTML doesn't even use kebab case for most attributes, much less all.


Trying to convert naming schemes was a bad idea, IMO, and will be removed in the next major. The types should only require that your observedAttributes all exist on the props obj.

@VelizarDemirev
Copy link

Hey I switched to my business machine. IMHO it would be nice to pass the propNames as kebab case since we loose the camelcase on the browsers. Here a quick example and how it looks in my browser:
image

image

as you can see firstName becomes firstname in the browser, IMHO it would look nice to have some kind of case here and if camelcase is not working then the next best would be kebab case.

@rschristian
Copy link
Member

rschristian commented Sep 15, 2025

That just how attributes work? They're always lower case.

div.setAttribute('FOO', 'bar') becomes <div foo="bar">

@VelizarDemirev
Copy link

Especially when it comes to longer prop names like in this extreme example:
image

then the props would be present in the dom like here:
image

@VelizarDemirev
Copy link

That just how attributes work? They're always lower case.

div.setAttribute('FOO', 'bar') becomes <div foo="bar">

sure but div.setAttribute('FOO-BAR', 'bar') becomes <div foo-bar="bar">too. ^^

@rschristian
Copy link
Member

rschristian commented Sep 15, 2025

sure but div.setAttribute('FOO-BAR', 'bar') becomes <div foo-bar="bar"> too. ^^

Add foo-bar to your component's prop types then, I don't get the issue here.

interface Props {
    firstName: string;
    first-name: string;
}

function MyComponent(props) {
    // ...
}

register(MyComponent, 'my-component', ['first-name']);

// <my-component first-name="Ryan"></my-component>

@VelizarDemirev
Copy link

sure but div.setAttribute('FOO-BAR', 'bar') becomes <div foo-bar="bar"> too. ^^

Add foo-bar to your component's prop types then, I don't get the issue here.

To have a better development experience? Now everybody has to convert the prop names internally like here:
image

or

image

knowing that you still handle this cases here in your index.js:

image

@rschristian
Copy link
Member

rschristian commented Sep 15, 2025

Yes, that's entirely intentional... you should explicitly declare which props your component takes. Don't write super long prop names if you don't want to type a lot I guess? I'm not really sure what to tell you, surely that's a universal issue, nothing specific to this?

As I mentioned, that props[toCamelCase(name)] will be going away in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants