GWTReact / gwt-react

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

We should restore the plain object annotation on ReactElement and DOMElement #13

Closed ivmarkov closed 6 years ago

ivmarkov commented 6 years ago

Currently, the GWT compiler in superdev mode issues strange cast exceptions when a local variable of type ReactElement is assigned a value.

I think the reason is as follows:

Currently, both ReactElement and DOMElement are classes and have the following annotation: @JsType(isNative = true)

However, the correct annotation should be: @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")

Why? Because ReactElement and DOMElement are plain JS objects. In other words, they do not have a prototype chain, and "instanceof" on ReactElement will return "object" rather than "ReactElement".

This is properly documented in React here:

ivmarkov commented 6 years ago

Maybe another viable alternative is to turn ReactElement and DOMElement into interfaces, because, as per the JSInterop spec, interfaces do not have an "instanceof" semantics. That would require to Move the type, props and key fields into getter properties - abreaking change. This however corresponds better to the fact that ReactElement is immutable (Object.freeze is being called right before a new ReactElement instance is returned).

ivmarkov commented 6 years ago

Not to put too much pressure here, but the issue is rather severe and is hitting us badly. I'm a bit surprised that you don't experience similar problems with GWT superdevmode.

pstockley commented 6 years ago

I am OK with the change, I just wanted to do some testing on our production codebase before committing. It's just unfortunate timing as we are just finalizing our monthly release this week so I won't have time until next week.

It's strange, that we have never run into this issue. Looking at our code base, we don't assign to variables of that type. We return these types from functions all the time but no assignments.