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

TypeScript (Help Wanted) #8120

Closed
tschaub opened this issue Apr 24, 2018 · 27 comments · Fixed by #10688
Closed

TypeScript (Help Wanted) #8120

tschaub opened this issue Apr 24, 2018 · 27 comments · Fixed by #10688

Comments

@tschaub
Copy link
Member

tschaub commented Apr 24, 2018

I'd like to see if we can publish TypeScript definitions for the 5.0 release. My understanding is that these can be published as @types/ol or in the ol package itself. I assume we'll need more than a single index.d.ts since we do not make everything exportable from a single module. I'm hoping we can automatically generate the TypeScript definitions from existing JSDoc annotations in the code. Perhaps this will be a migration path away from JSDoc annotations to authoring in TypeScript, but initially I think our goal should be to publish TypeScript definitions that are in sync with the code.

I'll be doing some more reading, but if anybody has experience and knowledge that they can spread, please let us know.

@tschaub tschaub added this to the v5.0.0 milestone Apr 24, 2018
@marcjansen
Copy link
Member

marcjansen commented Apr 24, 2018

@tschaub
Copy link
Member Author

tschaub commented Apr 24, 2018

@yairtawil - given your work on DefinitelyTyped/DefinitelyTyped#22894, I'm curious what you would suggest in terms of automating TypeScript definition files (dts-gen doesn't look that promising for ES modules).

@atd-schubert
Copy link

As long as you can also insert your TypeScript type definitions into your own module you should not apply them to DefinitelyTyped (DT). In addition there is already a definition on DT.

My recommendation is to insert a new definition based on the existing one with the help of the current definition author (@yairtawil). After that you can set the current type definition on DT as deprecated and maintain it in your module directly.

I can also help you a litte bit, if you wish.

@marcjansen
Copy link
Member

Please help, @atd-schubert. Your feedback is much appreciated.

@chrismayer
Copy link
Contributor

Wouldn't it make sense to have more members defined in the TypeScript types? In the Definition Files at https://github.com/DefinitelyTyped/DefinitelyTyped/ all classes are modelled like this

import * as ol from 'openlayers';

export default ol.layer.Vector;

Does it make sense to have the members like extent and visible defined there as well?

I just started with this TypeScript things, so sorry if I am wrong.

@atd-schubert
Copy link

Maybe another topic for our FOSSGIS Hacking-Event in Essen 😄

@beginor
Copy link

beginor commented Apr 29, 2018

maybe it is time to switch to typescript 😄

@atd-schubert
Copy link

@chrismayer of course you can just make an index.d.ts file that exports other type definition files maintained by different people for every class, if it that what you meant with your comment. I would strongly recommend doing this on big projects like OL.

The biggest advantage on TypeScript you will have when you switch to TypeScript completely like @beginor asked in his comment. TypeScript will check your source-code against contradictions. As long as you just write a type definition file TypeScript is not able to proof your theoretical type definition against the actual. In my current job I convert a lot of libraries to TypeScript and always found wrong architectural implementations during the rewrite process! TypeScript can not find them by a simple type definition, you will find them after you fail in a specific enough situation.

There is also a strong argument against TypeScript. JavaScript is undisputed the programming language with the biggest community. Switching to TypeScript minimize you potential community, especially for pull requests. On the other hand you will increase your code quality and stability that also has an influence on your projects community.

Writing a type definition in parallel has also the problem that you have to keep your definition up-to-date! So maybe someone can make a pull request easier in JavaScript, but on every change someone else have to check if there is also a change in the type definition! You have to develop your library in two different languages and it is on your responsibility that your codes will not differ!

So in the end the maintainers should ask themselves this question first: "Do you want to switch to TypeScript or do you want to maintain two languages in your project?"

By the way with TypeScript it is quite easy to switch step by step, because you can enhance a single file and keep the others in JavaScript, but you have to do it structured. For example in OL I would start with the event emitter, because it doesn't have any dependence. After that you can rewrite the files with dependencies on the event-emitter and so on...

@jumpinjackie
Copy link
Contributor

I've already made such a JSDoc plugin to auto-generate d.ts files from JSDoc documentation.

https://github.com/jumpinjackie/jsdoc-typescript-plugin

An example of openlayers.d.ts generated by this plugin:

https://github.com/jumpinjackie/jsdoc-typescript-plugin/blob/master/typings/openlayers.baseline.d.ts

I've been using it to generate typings for OpenLayers in my own projects for many releases now.

Despite the name suggesting it's a general purpose plugin, sadly it is not. It's just handles the OpenLayers case, which it handles very well.

@tschaub
Copy link
Member Author

tschaub commented Jun 22, 2018

Our current plan is this:

We're not going to block the 5.0 release on this, so I'll remove the milestone.

@fgravin
Copy link
Contributor

fgravin commented Jul 17, 2018

Hi, i just want to push back to a tool used by google to move from closure to typescript:
https://github.com/angular/clutz

  • goog.provide/require to import/export
  • functions to classes
  • closure JSDOC type annotation to typescript types

I think this tool could have been more used for all the tasked we've done for es6 module migration, maybe it can still be usefull and maybe people that were using openlayers and closure in their project could find this usefull.

@gberaudo
Copy link
Member

@mhosman
Copy link

mhosman commented Dec 14, 2018

No news about this??

@ahocevar
Copy link
Member

ahocevar commented Dec 14, 2018

The codebase is now annotated with JSDoc types that TypeScript understands. This work is complete, but there's one TypeScript issue that keeps us from running tsc type checks as part of our CI: microsoft/TypeScript#26883.

@joshferrell
Copy link

Is there a workaround for us to use in the meantime for our types, other than rolling back to v4? That bug seems to be getting kicked along and doesn't show any signs of getting resolved soon.

@idevtech
Copy link

What's the status of getting TypeScript definitions for v5.x now that TS v3.4.3 is released? Not being able to use TS for OpenLayers v5.x is holding up deployment of a project.

@hanreev
Copy link

hanreev commented Apr 26, 2019

i have manually generated typescript declaration fo openlayers v5.3.2 using JSDoc.
This declaration includes all documented API.

@stale
Copy link

stale bot commented Jun 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@fjorgemota
Copy link

I'm pretty sure this shouldn't be closed, right?

@ahocevar ahocevar reopened this Sep 13, 2019
@stale stale bot removed the stale label Sep 13, 2019
@ahocevar
Copy link
Member

ahocevar commented Sep 24, 2019

A regression in TypeScript 3.6 is currently affecting us: microsoft/TypeScript#33575.

@KaiVolland
Copy link
Contributor

Hey @hanreev,

I just forked your repository and it looks really great. 🎉

As we'd like to include the creation of .d.ts files within the CI-Pipeline we'd really appreciate if you could come up with some ideas or even a PR to include your templates and plugins within openlayers directly.

If you don't have the time and you don't mind I'm gonna try to do so.

@hanreev
Copy link

hanreev commented Sep 25, 2019

@KaiVolland with pleasure. that will be very helpful.

@KaiVolland
Copy link
Contributor

As @bampakoa state here #8448 (comment) the next typescript version will probably allow the generation of declaration files from js doc directly:

https://devblogs.microsoft.com/typescript/announcing-typescript-3-7-beta/
microsoft/TypeScript#32372

So we can easily create the .d.ts files with just adding declaration: true to the tsconfig.

https://media.giphy.com/media/rmi45iyhIPuRG/giphy.gif

@tschaub
Copy link
Member Author

tschaub commented Oct 7, 2019

I opened microsoft/vscode#82054 to describe the type of behavior I think it would be good to avoid. Having *.d.ts files published may make things nicer for TypeScript developers, but I think it would be unfortunate to lose the "Go to Definition" functionality and other features that let you navigate JavaScript sources in VSCode.

@stale
Copy link

stale bot commented Dec 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 6, 2019
@fjorgemota
Copy link

It's possible to add a tag (not sure which is more appropriate) so the stale bot doesn't mark this issue as stale anymore?

@stale stale bot removed the stale label Dec 6, 2019
@stale
Copy link

stale bot commented Feb 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

Successfully merging a pull request may close this issue.