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

Add TypeScript type definitions #24

Open
rwev opened this issue May 16, 2019 · 12 comments
Open

Add TypeScript type definitions #24

rwev opened this issue May 16, 2019 · 12 comments

Comments

@rwev
Copy link

rwev commented May 16, 2019

Here's a good start for leaflet-nontiledlayer.d.ts.

If this is something you'd like to do, let me know if a pull request is preferred.

@oliverheilig
Copy link
Member

Hi,
we currently have no plan to add TypeScript definitions to the package. I've read a thread regarding TypeScript with Leaflet.markercluster Leaflet/Leaflet.markercluster#725
I don't understand the exact procedere, but if you want to add the definitions to the package you can make a pull request.

@jonkoops
Copy link
Collaborator

@oliverheilig We're using this library in various places in @Amsterdam were we use TypeScript extensively. I'd be up for contributing not just type definitions but to re-write parts of the code in TypeScript itself and provide support on that code. This would replace the existing compilation with the TypeScript compiler and besides adding types also add support for ES2015 modules.

I'd love to know your stance on this as well, let me know if you'd be interested in this.

@oliverheilig
Copy link
Member

Hello,

i am currently not very active in this project anymore (and in web-development in general).

The question is how to continue the project without any breaking changes, especially for npm/unpgk.

So the options would be that i grant you full access, or you just make a fork with a different name.

Oli

@jonkoops
Copy link
Collaborator

jonkoops commented Nov 20, 2020

The conversion should not introduce any breaking changes, if you would like to add me as a maintainer that would be great. If the maintenance of this library is too much I can also inquire to move it to the Amsterdam organisation for maintenance.

For now I think adding me as a maintainer would be a good start 👍 I'll make sure to open up some PRs so that the changes can be tracked.

@oliverheilig
Copy link
Member

OK, i granted you access as a contributor. But i also had some thoughts.

Converting the source base iteself to TypeScript would be a major change. It would also go against the initial "Leaflet philosophy" as a minimalistic vanilla-JS component.

The source didn't really change over the years because the component does the job and is somehow "complete". I am not sure if this major change would be in the best interest of all developers.

Maybe we can get more input from other people.

@jonkoops
Copy link
Collaborator

@oliverheilig I've started by opening some pull requests for quality of life improvements and added you as a reviewer. Let me know if this is how you would like to approach this, or if you are perhaps fine with letting me merge these as I see fit (in which case I will let some of my colleagues at Amsterdam peer review it).

In regards to introducing the TypeScript compiler, I have every intention to try and ship this as a non-breaking change for consumers of this package. This would be done by providing the source code as TypeScript and compiling it to JS, which is what will end up in the resulting package.

If there is no longer interest for maintaining this library from your person (or organization) I could inquire if we at Amsterdam would be willing to take on the burden of maintenance and development. Which would involve moving this repository to the @Amsterdam organization and transferring NPM publish rights to our account.

Let me know your thoughts!

@atlantageek
Copy link

Any update on this. Is there a typings file I can use. I saw a reference to one at the beginning of the issue but the link is broken now.

@jonkoops
Copy link
Collaborator

jonkoops commented May 24, 2021

See #45, I am adding support for Typescript there. It does not yet include the type definitions as I want to convert the source as well in a future PR.

@atlantageek
Copy link

Any updates on this. I would really like to use this in my angular project.

@jonkoops
Copy link
Collaborator

Any updates on this. I would really like to use this in my angular project.

Haven't had free time recently to be working on this unfortunately. We can compile the source as TypeScript now, so feel free to open up some PRs to improve the type-safety.

@sanderpukk
Copy link

Any chance we can get a release with the current Typescript? I see there was something done #45 but release is from March 20th.

@jonkoops
Copy link
Collaborator

@sanderpukk The current version on main contains breaking changes and there are not yet any type definitions that are being emitted. Doing a release at this point is not going to resolve this issue.

If you'd like to see this issue resolved faster feel free to open up some PRs to improve the type-safety of the code.

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

5 participants