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

OL3 API externs #15

Closed
klokan opened this issue Jul 30, 2014 · 11 comments
Closed

OL3 API externs #15

klokan opened this issue Jul 30, 2014 · 11 comments
Assignees
Milestone

Comments

@klokan
Copy link
Member

klokan commented Jul 30, 2014

For the compilation of the ol3-cesium in advanced mode independetly of ol3 we must have externs.

It seems there is no way how to generate externs for ol3 API right now. (It is not to be mixed with exports).

Closure compiler wrapped in node.js does not support it - it is not parameter of the compiler on the command line either).
Plovr can do that, but it seems jsdoc types are not passed trough.

Ideal way how to generate Closure exports for public OL3 API would be probably via a custom JSDOC template.

Required is:

  • new jsdoc template to automatically generate ol3 api externs for everything with @api
  • modification of the npm/node powered compilation system to support "generate-externs" task producing the externs

Because we don't have externs for OL3 API we compile now in WHITESPACE :-(.

Alternative would be to compile ol3-cesium together with ol3 itself - to get a single ADVANCED minimised js, which we plan to support anyway, but it does not follow the preffered external ol3 approach we specified before.

@ahocevar
Copy link
Member

I'm on it. Should be done by the end of the week.

@klokan
Copy link
Member Author

klokan commented Jul 30, 2014

Great. Thank you!

@ahocevar
Copy link
Member

See openlayers/openlayers#2479.

@klokan
Copy link
Member Author

klokan commented Aug 5, 2014

ol3-cesium can be now compiled in ADVANCED with the new externs (but only with checkTypes: "OFF" because of issues with missing @api annotations in ol3 code).
Thanks @ahocevar and @petrsloup

@klokan klokan closed this as completed Aug 5, 2014
@elemoine
Copy link
Member

elemoine commented Aug 5, 2014

Alternative would be to compile ol3-cesium together with ol3 itself - to get a single ADVANCED minimised js, which we plan to support anyway, but it does not follow the preffered external ol3 approach we specified before.

If we want to compile ol3-cesium with Closure Compiler in advanced mode I do not understand why we do not always compile ol3-cesium and ol3 together. And if we do that we do not need externs for ol3 and we can use in ol3-cesium any ol3 properties/symbols – no need for exports.

@ahocevar
Copy link
Member

ahocevar commented Aug 5, 2014

@elemoine FYI, it is one of our requirements to have a stand-alone build of ol3-cesium.

@elemoine
Copy link
Member

elemoine commented Aug 5, 2014

@elemoine FYI, it is one of our requirements to have a stand-alone build of ol3-cesium.

Yep, that makes sense. But still. For the stand-alone build all the ol3 exports could be included. In this way ol3-cesium.js would effectively the full ol3. And if what you is a subset of ol3 you can even define your own set of ol3 exports in the build configuration file.

@ahocevar
Copy link
Member

ahocevar commented Aug 5, 2014

By stand-alone build I mean a build of ol3-cesium, without ol3 or cesium included. The npm way. Both ol3 and cesium are included separately.

@elemoine
Copy link
Member

elemoine commented Aug 5, 2014

By stand-alone build I mean a build of ol3-cesium, without ol3 or cesium included. The npm way. Both ol3 and cesium are included separately.

Ok. Out of curiosity why wouldn't a standalone ol3-cesium build that does include ol3 work for you? People using ol3-cesium would need a full ol3 build anyway, because they would not be able to know what of ol3 ol3-cesium would need for its own use.

One of our requirements would be to be able to compile ol3 + ol3-cesium + app together. Do you agree with this requirement as well? I think this requirement is compatible with yours. I'd just find it regrettable that we wouldn't be able to use non-exportable ol3 symbols/properties in the ol3-cesium code. For a close integration having that capacity would be useful in my opinion.

@ahocevar
Copy link
Member

ahocevar commented Aug 5, 2014

Let's avoid using non-exportable ol3 symbols in ol3-cesium. If we find that something we need for ol3-cesium is not exported in ol3, I'm sure we'll be able to improve the ol3 API to make it work with ol3-cesium. I think this is an approach we had agreed on already.

Other than that, there are of course no objections to compile ol3 + ol3-cesium + app together.

@elemoine
Copy link
Member

elemoine commented Aug 5, 2014

Let's avoid using non-exportable ol3 symbols in ol3-cesium. If we find that something we need for ol3-cesium is not exported in ol3, I'm sure we'll be able to improve the ol3 API to make it work with ol3-cesium.

I think this adds friction to the development, that is why I would suggest to do differently. And I do not see any problem with providing a standalone ol3-cesium that includes ol3 (as people using ol3-cesium would need a full ol3 build anyway).

When updating ol3-cesium with a new version/tree of ol3 the compiler would catch it if ol3-cesium references symbols/properties that do no longer exist in ol3.

I think this is an approach we had agreed on already.

In the context of the ol3-cesium development? I must have missed that discussion.

Other than that, there are of course no objections to compile ol3 + ol3-cesium + app together.

Ok cool. So we'll need to make sure that it works.

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