Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Depend directly on Elemental2 #77

Closed
dmg46664 opened this issue Apr 12, 2017 · 10 comments
Closed

Depend directly on Elemental2 #77

dmg46664 opened this issue Apr 12, 2017 · 10 comments

Comments

@dmg46664
Copy link

My problem is that because the existingJsArray is defined as JsArray<T extends JavaScriptObject> extends JavaScriptObject, it is enforcing too harsh a restriction the items stored in the array. I don't want to define my class as extending JavaScriptObject as this is a deprecated API and would prefer my @JsType annotated classes not to have all the restrictions imposed by the compiler.

I propose creating a JsArrayNative type that is included in this generator project which would work as an expected javascript array without any of the restrictions of JsArray. I didn't want to just go ahead and create this and submit a pull request if this is not the preferred approach of dealing with this. Obviously this would be a breaking API change to the generator.

What are your thoughts?

@manolo
Copy link
Owner

manolo commented Apr 12, 2017

Well, since elemental2 preview is out, probably the next steps in the project would be get ride of temporary implementations in the project and use those. I have not taken a look to elemental2 but probably it does not have these problems.

@dmg46664
Copy link
Author

@manolo Thanks for the steer. I will take a look at what is on offer there.

If nothing is available perhaps keeping the name JsArray with the same API and different package name to minimize code breakage... but I am getting ahead of myself.

@dmg46664
Copy link
Author

@manolo - Nice! There is elemental2.Array<T> which looking superficially looks like it solves the problem 👍

I used the following link to investigate: https://oss.sonatype.org/service/local/repositories/google-releases/content/com/google/gwt/elemental2-experimental/16-06-30/elemental2-experimental-16-06-30.jar

Will see if I can try and get it working with:

<dependency>
   <groupId>com.google.gwt</groupId>
   <artifactId>elemental2-experimental</artifactId>
   <version>16-06-30</version>
</dependency>

@manolo
Copy link
Owner

manolo commented Apr 12, 2017

Good to know. Thanks for reporting.
There is no so much documentation related with this, but you can take a look to this thread where elemental2 beta was announced https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/google-web-toolkit-contributors/MyGZ4-ASG-Y/PVuZvzwwDAAJ

@dmg46664
Copy link
Author

Thanks again for setting me in the right direction.

@dmg46664 dmg46664 changed the title Replace JsArray with JsArrayNative Replace JsArray Apr 12, 2017
@dmg46664 dmg46664 changed the title Replace JsArray Depend directly on Elemental2 Apr 12, 2017
@dmg46664
Copy link
Author

@ibaca
Copy link

ibaca commented Apr 16, 2017

Are you sure that it is not enough to depend on jsinterop:base? using JsArrayLike for example?

@dmg46664
Copy link
Author

dmg46664 commented Apr 17, 2017

@ibaca - No, I'm not sure and that's probably a better idea :-)

I was thrown a little by the name *ArrayLike, as if it had some other purpose besides just declaring the interface. More canonical for the interface to get the name and the class to be *Impl given what I'm used to.

@dmg46664
Copy link
Author

@ibaca - My test was to use the gwt-api-generator with Vaadin's new grid 2.0. I was only trying to hack it to get it working. It appears to work.

Obviously I haven't tested it with JsArrayLike. Presumably this would work fine provided you passed in an Array when expected. I guess ideally the generator would generate the most specific typed interface possible, in the case of the grid stating specifically that an Array is required ?

@ibaca
Copy link

ibaca commented Sep 9, 2017

This seems to be solved! 👏

@manolo manolo closed this as completed Sep 10, 2017
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

No branches or pull requests

3 participants