search

alex-kinokon / jsx-dom

Use JSX to create DOM elements.
BSD 3-Clause "New" or "Revised" License
277 stars 30 forks source link

key attribute #73

Open KapitanOczywisty opened 2 years ago

KapitanOczywisty commented 2 years ago

Currently key attribute is silently skipped, shouldn't this be set as attribute or property or at least in dataSet?

https://github.com/proteriax/jsx-dom/blob/a24e63e96836575ddf878a2a43263c7178c86db1/src/jsx-dom.ts#L135-L136

Kir-Antipov commented 2 years ago

shouldn't this be set as attribute or property or at least in dataSet?

Nope, it should not:

Keys serve as a hint to React but they don’t get passed to your components. If you need the same value in your component, pass it explicitly as a prop with a different name

source

KapitanOczywisty commented 2 years ago

This is not react and key have to be accessible somehow to be any useful, for example to use with morphdom. Otherwise should be removed from types.

alex-kinokon commented 2 years ago

jsx-dom strikes to emulate React’s API and behavior as much as possible, that’s why key is stripped away.

KapitanOczywisty commented 2 years ago

And another thing to change in patch-package...

Completely removing key is wasteful, since it could be used with packages like morphdom, especially when somebody consciously adds key attribute to the code. Of course I could use id or class or dataset, but 1. this is inconvenient 2. some tools like eslint have special rules for key. Keeping key as at least dataset['jsxKey'] or data-jsx-key would make way more sense than silently throwing it away. Same goes for capture variants of events - PR is waiting for feedback.

jsx-dom strikes to emulate React’s API and behavior as much as possible

However some additional features are present, this could be one of them.

Edit: In #17 you already accepted change to divert from react-like implementation, also for morphdom use case.