search

preactjs / preact-compat

ATTENTION: The React compatibility layer for Preact has moved to the main preact repo.
http://npm.im/preact-compat
MIT License
949 stars 148 forks source link

auto alias className to class will make props trustless #422

Closed paranoidjk closed 5 years ago

paranoidjk commented 7 years ago

Reproducible demo: https://codepen.io/paranoidjk/pen/MvdLpd

This is caused by https://github.com/developit/preact-compat/pull/412, it breaks our code base, please revert it.

The reason is preact-compat add a extra alias attribute class to component props when it has className attribute.

But in some case, we will want to omit className and pass rest props to child component, in this case, child component will got a unexpected class attribute.

I think preact-compat can not require users to delete class and for when they want to omit className, htmlFor, since preact-compat goal is to compatibility with react but no need to change users code when they want to switch form react to preact.

paranoidjk commented 7 years ago

cc @developit

developit commented 7 years ago

What if we made it non-enumerable?

paranoidjk commented 7 years ago

@developit It probably ok. Since we normally won't access props.class directly in react codebase, and babel will compile spread operator to use Object.assign or a polyfill with for in, in this case it should works fine .

But i am not sure it will be 100% safe. So i think there is a tradeoff.

IMO, i suggest preact-compat avoid to pollute users component props object in runtime, it's a little dangerous.

marvinhagemeister commented 5 years ago

I'm unable to reproduce the described issue with Preact X anymore :tada: