Gradually migrating to TypeScript

[chrisvanmook]

Is your feature request related to a problem? Please describe. As I don’t encounter any bugs at the moment, I am slightly worried about future maintenance of Leaflet, as I want to use Leaflet in my applications for at least a couple of years. I’ve noticed that the code in some files are still written in old JS syntax and types need to be maintained separately. A mismatch between the types and the actual code could introduce some issues.

Describe the solution you’d like Gradually migrating to TypeScript would be a solution to start with. Why?

• Able to use modern syntax while still supporting older browsers
• All the benefits of TypeScript
• If everything is converted to TS, the @types/leaflet package is no longer needed
• Overal better development experience

Describe alternatives you’ve considered A solution with minimal impact on the build process and code. Being able to actually use TypeScript and converting existing JS files to TS would be a good start. I’ve quickly tried something out, seems that the build output is almost the same https://github.com/chrisvanmook/Leaflet/pull/1/files.

I would like to know if you, the maintainers / contributors of leaflet, would be open for a similar change like this? In that case I will work on a pull-request, I might need some help with getting karma and leafdoc to work with TS.

[IvanSanchez]

I would like to know if you, the maintainers / contributors of leaflet, would be open for a similar change like this?

I'm very wary of TS. Specially when some of the Leaflet code still works around browser quirks, and I fear that TS will try to enforce a strict definition of properties available in document, window and so on. The little I've had to work with TS, it got in my way and felt like a burden.

Ideally I'd like work to be done on browser-loadable ESM modules (developing without needing to run a bundling toolchain feels so good), move the examples to <script type=module> and deprecate IE10 functionality (along with L.SVG.VML). I'm personally against complicating the toolchain.

[chrisvanmook]

@IvanSanchez great to hear you want to move towards ES Modules! Do you have a roadmap / plan for this? Im curious. Regarding your worries about TypeScript, I'm not sure I fully understand. TypeScript indeed enforces the developer to use type definitions, that is it's purpose and that's a good thing, right? In some rare cases where this could be an issue and blocks you, you could always use ts-ignore or cast it as any or unknown and fix it later. Perhaps in the current situation, where the Leaflet object is exported on window, it might be a little bit of an issue. However, when moving towards ESM, TypeScript could definitely help! I'm not sure what the plans are, but you could start using TypeScript for the new ESM and still keep the current setup for the current js files.

[mourner]

I might have an unconventional taste in programming with some unpopular opinions, but I'd like to avoid adopting TypeScript in Leaflet. It has always been intended to remain a simple, minimal project, unburdened by complex type systems, heavy transpilation toolchains and fashionable programming paradigms.

Types can be extremely beneficial in huge end-user projects, but I've found them burdensome when maintaining libraries, adding verbosity, tricky integration issues, making the code harder to skim, adding pressure to upgrade to newer tool versions, and (especially in more dynamic areas) making me fight the type system so much that it sucks all the joy out of programming.

For future, I share Ivan's thinking in moving towards ESM and "what you see is what the browser evaluates".

[simon04]

Ideally I'd like work to be done on browser-loadable ESM modules

What is left to be done? Importing a bunch of Leaflet JavaScript source files inside a <script type="module"> works without problems: https://gist.github.com/simon04/21ae8e3d07ec7f6b62ba5dcad403cc4a

[IvanSanchez]

What is left to be done? Importing a bunch of Leaflet JavaScript source files inside a <script type="module"> works without problems

IMO, documentation. Edit the docstrings to add a new example section to all classes showing the import statement. Change the downloads.md webpage template. Add a tutorial about the issue.

Also, the trouble with paths. There's gonna be two kinds of Leaflet users: those who use bundlers (rollup, webpack, vercel et al) and those who don't (relying on vanilla browser imports). Depending on that, the import paths are gonna be different, and I don't have a clear idea of what the final look of that documentation would be like.

[jonkoops]

In regards to TypeScript support, instead of adopting it for the source, it could also be supported by embedding the type definitions to the library itself in the form of type declaration (d.ts) files. This would also simplify the process for users of TypeScript and library authors (also if they just want IntelliSense in their IDE and are not actually using it in their stack) as they would no longer have to install a seperate @types/leaflet package.

What is the opinion of the maintainers on this one?

[backspaces]

Please! No! Not to be mean, but really, TS has become the bully on the playground. When it is supported by browsers, then yup.

[jonkoops]

Not to be mean, but really, TS has become the bully on the playground.

It solves genuine problems that people experience, but if it's a good fit for this project is a different discussion. The fact is that the Leaflet types are downloaded over 225.000 times a week, which is half of the downloads of Leaflet itself. This means that 50% of the Leaflet (NPM) user-base is also using TypeScript in one way or another so they seem to think it's pretty useful.

Adding type definitions (not TypeScript itself) to the NPM package will make it easier for that 50% to do their job, and it has no impact whatsoever for those who do not use it. In fact even if it was decided to allow TypeScript in the code itself, it would still not impact you, since the compiled code is in fact just plain JavaScript.

When it is supported by browsers, then yup.

This will never happen, and should not be a criteria for getting compile time guarantees about the correctness of code.

[jonkoops]

@mourner @IvanSanchez Can I get your opinions on this? As a TypeScript user like many out there it would be hugely beneficial for us to have the type definitions out of the box with the NPM package. Meaning only adding type definition files which live next to the source files, without touching them or introducing any complex compilation or other transformation steps.

[chrisvanmook]

Types can be extremely beneficial in huge end-user projects, but I've found them burdensome when maintaining libraries, adding verbosity, tricky integration issues, making the code harder to skim, adding pressure to upgrade to newer tool versions, and (especially in more dynamic areas) making me fight the type system so much that it sucks all the joy out of programming.

I'm aware of the fact TS is an extra dependency and has a learning curve if you start working with it, in the beginning this could indeed be frustrating. However, I think having a leaflet-package that would reduce the chance of potential runtime type-errors (and thus reduce defensive coding), will make the whole package way more robust, easier and saver to work with in projects. This, in my opinion, offers way more value than having a "better" developer experience (which is of course also debatable, some developers embrace TS, some could find it somewhat of a burden). By the way, it always compiles to JS so the package just could be used in regular JS project, but I’m sure you where already aware of this. Your point of using ESM (and not transpiling code) is a valid argument though, I'm just wondering what would be more important in regards to type safety and developer experience. I think eventually the point here is to improve the end-product. I'm curious what your opinion is about this?

[johnd0e]

As a TypeScript user like many out there it would be hugely beneficial for us to have the type definitions out of the box with the NPM package.

Note that "Leaflet types" are maintained by completely different people.

No one from current Leaflet support team is able to help with that. The best we can do - add some manual article referring to "Leaflet types".

[jonkoops]

Note that "Leaflet types" are maintained by completely different people.

As one of the contributors to the types package for Leaflet I am more than willing to put that effort here instead. And I am very sure others also do not care where they contribute these types as long as they can use them and yield the benefits.

[chrisvanmook]

The TypeScript compiler can also be used to generate type definitions from JSDoc: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html

Perhaps this could be considered a good alternative?

For example:

/** @typedef {{ lat: number, lng: number }} LatLngLiteral */

/**
 * @param latLng {LatLngLiteral}
 */
export function someFn(latLng) {
  console.log(latLng.lat)
}

Would generate the following type definitions:

export type LatLngLiteral = {
  lat: number;
  lng: number;
};
/** @typedef {{ lat: number, lng: number }} LatLngLiteral */
/**
 * @param latLng {LatLngLiteral}
 */
export function someFn(latLng: LatLngLiteral): void;

[backspaces]

OT: This is interesting! I am adding jsDoc comments to a repo now, to generate documentation via documentation.js.

Does this mean, if I do it right, I can become "TypeScript friendly" so to speak?

Would I need to add a workflow step, turning the comments to something TS consumes?

[chrisvanmook]

@backspaces Yes, the only workflow you need to add is the typescript compiler to generate the *.d.ts files from the .js files when publishing a new package (together with minimizing / bundling a .js file). With this we can kill 2 birds with one stone: having types generated from the source, and even better documentation. The only downside to this is that we depend more on our IDE / editor supporting JSDoc and type definitions via JSDoc (or we have to check it very carefully ourselves, which is far from ideal). It would also be less strict as well than having actual TS files.

[mathe42]

I think there is a more important step / a step that has to happen before migrating to tyescript:

To migrate this custom class syntax and use the es6-class syntax. It can be transpiled down with no problems.

I'm currently working on it.

[chrisvanmook]

I think there is a more important step / a step that has to happen before migrating to tyescript:

To migrate this custom class syntax and use the es6-class syntax. It can be transpiled down with no problems.

I'm currently working on it.

Nice to hear you work on this! Perhaps while refactoring, you could keep this in mind, like introducing type definitions in JSDoc, if you want to :)

[backspaces]

I believe TS is es6 module based, right? I ask because https://github.com/Leaflet/Leaflet/issues/7055 has a conversation on es6 modules.

In particular, those of us using modules cannot use the leaflet style of assigning plugins to L due to the es6 version of L is a module, thus fails doing L.ElementOverlay = L.ImageOverlay.extend({ (ElementOverlay is a plugin our team developed).

Our solution is to simply use the usual module approach:

import * as L from 'https://unpkg.com/leaflet/dist/leaflet-src.esm.js'
const ElementOverlay = L.ImageOverlay.extend({
...
export default ElementOverlay

But there is not agreement amongst us Leaflet users on a standard approach. Perhaps TS would suggest the best approach? One that we could all use, TS users or not?

[mathe42]

Nice to hear you work on this!

Might not be clear: I'm not a Team member of Leaflet.

But there is not agreement amongst us Leaflet users on a standard approach. Perhaps TS would suggest the best approach? One that we could all use, TS users or not?

I think it is a BAD habit to put all of Leaflet on the window object. For following reasons:

1. No Treeshaking
2. No dynamic loading after first render of webpage

And if you use a Syntax it is a bad thing to let Plugins modify it. I think the best way for Plugins is the following:

import { ImageOverlay } from 'https://unpkg.com/leaflet/dist/leaflet-src.esm.js'
const ElementOverlay = ImageOverlay.extend({
//...
})
export default ElementOverlay

or

import { ImageOverlay } from 'https://unpkg.com/leaflet/dist/leaflet-src.esm.js'
export default class ElementOverlay extends ImageOverlay {
   //...
}

The second syntax will have problems if you extend options. But you get full typings!

[jonkoops]

Perhaps TS would suggest the best approach? One that we could all use, TS users or not?

In this regard TypeScript has the same constraints, as it is compiled down to JavaScript.

[mathe42]

Just for progress tracking: https://github.com/leaftypes/Leaflet/pull/2

I have more rewritten but that rewrite was not bugfree so know I do it file by file and run all tests.

I currently build 3 Targets and run the tets (in a modern browser) for all 3:

legacy = non es6 with class sytax we need to pollyfill Object.create globaly or we loose some IE versions. (I think <= 9 or so) es6 = es6 classes modern = es2017 syntax supportet by 95% of Users.

[mathe42]

see #7614 for a PR to this Repro for some feedback!

[jonkoops]

@mathe42 Nice that you are working on this, but perhaps it's a separate issue from this one as the class syntax is unrelated to improving support for TypeScript users. Perhaps you can create one to start the discussion there?

[nesterenko0602]

@IvanSanchez

IMO, documentation.

Given that the code is (almost) done, whether it possible to publish that branch where the work is in progress? Really looking forward to ESM leaflet and wanted to try it ASAP as it would solve some of the performance related problems in our project.

[jonkoops]

I think this might be a useful feature that we could consider for 2.0 so I added it to the milestone. This does not mean this is automatically accepted.

[kamoshi]

One positive aspect of including TypeScript compiler in the tool chain is that it allows you to target ES5 while at the same time being able to use most of the modern syntax provided by TypeScript.

I was able to build Leaflet using TypeScript compiler to some success on my own, but it would be a lot of work to port the entire codebase. In my limited tests I rewrote parts of the Utils.js file in TypeScript, and it seemed to integrate seamlessly with the rest of the codebase which remained in JS.

[jonkoops]

One positive aspect of including TypeScript compiler in the tool chain is that it allows you to target ES5 while at the same time being able to use most of the modern syntax provided by TypeScript.

True, although this also could be accomplished with a plugin for Rollup like babel. We are moving to a more modern ES2015+ syntax by default so in the future there should be no need to transpile to older editions.

And yeah, if we were to adopt TypeScript we'd be doing so incrementally. A complete re-write of all the code is not a very feasible scenario.

[backspaces]

Type Annotations seem to be a more interesting alternative, requiring no build step, and a potential standard ecmascript feature in stage 0 https://blog.logrocket.com/types-as-comments-strong-types-weakly-held/

[jonkoops]

Indeed, I have high hopes for type annotations becoming standardized. This would make it a lot easier to build libraries with support for Flow/TypeScript, without the need to have a compilation step.