GWTReact / gwt-react

GWT Java bindings for React
MIT License
91 stars 17 forks source link

Did we overtype ReactElement? #17

Closed ivmarkov closed 6 years ago

ivmarkov commented 6 years ago

Currently, ReactElement has the following signature:

public class ReactElement<P extends BaseProps, T> {
    public T type;
    public P props;
    public String key;

    /**
     * Objects of this class cannot be directly instantiated by the user.
     */
    protected ReactElement() {
    }
}

That's all nice and dandy, except that every time we create or pass around ReactElement instances, we find ourselves always using the ReactElement<?, ?> signature. In other words we don't care a single bit what the actual types of the Props class and the component are. Not to mention that the T parameter is unbound, and cannot be make bound, because ComponentConstructorFn, StatelessComponent, and Component all have no single parent and cannot have one.

I actually have my own personal contribution in the above (remember?), but perhaps it is not too late to consider alternatives. How about this:

public class ReactElement {
    public Object type;
    public BaseProps props;
    public String key;

    /**
     * Objects of this class cannot be directly instantiated by the user.
     */
    protected ReactElement() {
    }
}

Yes. We lose a bit of type safety for the rare case when somebody would indeed have to be sure what the exact type of the props and component type is, but how frequent are these cases?

pstockley commented 6 years ago

99% of people wont really care about the implementation details of ReactElement. I am ok changing it. However, it is going to be a breaking change. My plan is to release react 16 support first. Then I will do one release with all the breaking changes. That way people don't have to keep migrating their codebase.

pstockley commented 6 years ago

Given that I am in the middle of a big refactor to support React 16.2 and Elemental, I committed this change. I also removed DOMElement as it just subclassed ReactElement and added no further value.