Migrate to TypeScript

[nyurik]

I would like to propose this project is migrated to TypeScript as a goal for v2 or v3 release.
Partially relates to #24

[curran]

Why?

[curran]

This already exists https://www.npmjs.com/package/@types/mapbox-gl

[nyurik]

This already exists https://www.npmjs.com/package/@types/mapbox-gl

This is not the same -- the @types/* is created for the benefit of the users of the library. Converting the library itself will help the developers of that library (all of us) to spot bugs early on. In short, if somewhere deep in the library we have function foo(x) {...}, the current compiler may not catch the case of calling that function with a wrong parameter type. Changing it to typescript will force us to specify the type and for callers to match that type - greatly reducing the potential for a bug.

The other important benefit of converting to typescript is that it will force us the maintainers to go through all of the code, thus gaining better understanding of how it works.

[msbarry]

If the project were starting in plain javascript, I might question the effort of moving to typescript. But since it's starting with flow, I think this makes sense (but will still probably be a large chunk of work). There are likely very few consumers of flow types, and a lot of consumers of the separate type definitions over in https://github.com/DefinitelyTyped/DefinitelyTyped - after the migration it would be less work to just maintain typescript definitions here.

A quick "flow to typescript" google search pulls up some migration utilities and first-hand accounts.

[rbrundritt]

This would be awesome. I use javascript and typescript almost daily and for anything I work on for production I prefer to use typescript. I've caught so many my bugs in code before releasing because of its type safeness. This would make changes like getting feature state working with string id's a lot easier.

It is also possible to compile typescript apps I to web assembly which may be useful in the future.

[curran]

Converting the library itself

I'm curious, how much effort would it take to convert the codebase to TypeScript (in terms of developer hours)?

some migration utilities

Might be worth an initial investigation to run one of those on this codebase and see what happens!

[folke]

fyi: I've just created maplibre-gl typings at definitelytyped: DefinitelyTyped/DefinitelyTyped#50116

It's basically just a copy of the mapbox-gl 1.13 types, with updated package names.

To speed up having this on npm, it would be great if anyone could review: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/50116/files

[folke]

I've played with a couple of conversion tools this evening and flowts gave the best results.

If you're interested, have a look at my fork

Things I did:

• remove flow
• added typescript
• confgured tsconfig
• updated config for eslint
• added prettier
• tsconfig currently has strict:false to make it a tiny bit more manageable 😄
• moved some fllow-typed typings to .d.ts
• fixed a bunch of errors

I've added two script targets for testing:

• yarn maplibre:ts will run tsc
• yarn maplibre:lint runs eslint on src

All syntax errors have been fixed, but as to be expected, there's about 1300 typing errors left right now.

[nyurik]

@folke this is awesome, thank you! One thing that immediately catches the eye -- too many unneeded changes. I believe we should try to minimize the number of churn in such a big migration as it makes it nearly impossible to review, e.g. keep the style the same if possible -- do not delete semicolons, etc. This way git will show these as renames rather than delete+add. I think the typescript eslint rules should be near the same as for the javascript code. Additionally, I am a big fan of classical styling, i.e. proper semicolons everywhere, trailing commas in multiline lists, etc -- we are not trying to optimize for code size, we should optimize for human readability and ease-of-review.

My biggest reservation at the moment (other than the difficulty in reviewing and testing) is that there might be some community pull requests under BSD license that we may want to merge, but we would have to do it before typescript migration or else merging them would become too difficult. I wonder if there are any such PRs pending.

Once again, so glad you are hacking on it!!!

[mike-marcacci]

Hi all. Just wanted to reference a similar endeavor to migrate a large open source project from flow to TypeScript. I'm intermittently active in the GraphQL ecosystem and various related working groups, and we have had a longstanding effort to rewrite graphql-js into TypeScript.

This project has been in the works for multiple years now and has had many false starts, where an attempt encountered some difficulty and stagnated. The challenges that killed these attempts weren't technical incompatibilities but related to process and change tracking. The issues @nyurik already brought up are profoundly important, and very difficult to address:

1. reconciling other concurrent changes with a rewrite
2. preserving legible change history
3. ensuring the rewrite is reviewable

Here are a few "pro tips" from our learnings, which might be useful here:

• Commit to a feature freeze; it's completely impractical to have any other major changes in the works while this project is under way.
• Merge any desired runtime changes before the feature freeze, and make as few runtime and control flow changes as possible during the rewrite even if the resulting code is not perfectly idiomatic TypeScript; these can be fixed later.
• Rename all files from .js to .ts in a single commit without any content changes; otherwise git will have difficulty associating the renamed files and will count them as file deletions and additions, breaking their history.
• Consider converting each language feature in its own commit; this has proven to significantly help reviewers make sense of the massive number of changes.

[folke]

I totally agree to both of you! As I said, I just set this up to learn more about what the migration process would entail once it would get started (happy to help on that once the time is right).

The biggest reason I integrated prettier, was because the first two conversion tools I used totally messed up the original formatting (they generate the source based on the AST without preserving formatting) and I couldn't get eslint's formatting to work initially.

Flowts actually uses recast to try to preserve formatting as close as possible.

As you pointed out, the first commit should definitely be a rename of the .js files. Or even better, an explicit git mv for each file.

What I 'll do is, I'll set up a clean fork of maplibre-gl, where I'll add a single script that can convert and setup the codebase properly for anyone else that wants to play around with this.

[folke]

My branch now has a clean fork without transpiled ts files, only the minimal changes for eslint, typescript and a migration script.

To test, checkout the banch, run yarn install and yarn maplibre:migrate. This will:

• install typescript
• install @types for all dependencies
• run flowts to convert all js files under src to ts files
• copy converted module typings from ts-typed to src/types (I manually fixed those that were needed from flow-typed)
• yarn maplibre:lint:fix that fixes some formatting issues from the transpiler based on the original Mapbox formatting settings

[folke]

I promise I'll stop spamming after this, but definitelytyped just released the @types/maplibre-gl package with mapbox 1.13.0 types 🎉

[stevage]

I know there's a lot of enthusiasm for TypeScript, but:

• this raises the barrier for new contributors (believe it or not, most JS developers don't know/use Typescript)
• Turf went down this path before, and it caused a lot of pain, and not much benefit. Getting Turf back on track Turfjs/turf#1428

Is this really an essential feature to implement right now? What exactly are the benefits? What's the downside of not doing it?

[msbarry]

Since the existing code uses flow throughout, this is more of a typescript vs flow question than typescript vs JavaScript. I think that most of the cost was paid when they initially added the flow types that exist in the codebase today but @folke could give a better estimate of remaining effort to migrate.

[easbar]

What I am missing a bit in this discussion here is a clear explanation how such a migration shall be justified (taking into account the effort that is needed and some of the other drawbacks that were mentioned). So far I see:

Converting the library itself will help the developers of that library (all of us) to spot bugs early on. In short, if somewhere deep in the library we have function foo(x) {...}, the current compiler may not catch the case of calling that function with a wrong parameter type. Changing it to typescript will force us to specify the type and for callers to match that type - greatly reducing the potential for a bug.

Type safety has its benefits, sure. But so does dynamic typing and both are very popular. And as it was pointed out already, Mapbox already uses flow for type checking. Apparently, the Mapbox team decided for this and against TypeScript and probably for some reason?

The other important benefit of converting to typescript is that it will force us the maintainers to go through all of the code, thus gaining better understanding of how it works.

Sure doing some migration can help forcing yourself to look at the code, but so far a mostly automatic conversion was discussed and no one holds you back from reading the code without changing it. I do not have significant TypeScript experience, but maybe it would be better to get familiar with the code first and then decide whether the migration is a good idea?

To me currently the most important goal of this project seems to be the longer term maintenance of Mapbox 1.13 with an open license. It seems like a sound library but there might be bugs and some browser incompatibilities might show up in the future and for users of Maplibre it will be important that these issues will be addressed reliably. I might be wrong, but so far there is no clear roadmap for adding new features, so I am really wondering why migrating to TypeScript is one of the first things being discussed here.

I am also wondering: Will switching to TypeScript make it any harder for users to migrate from mapbox 1.13 to maplibre? This would be a huge point against this in my opinion.

[folke]

This is indeed more a question of what language we want to use to maintain maplibre-gl. The current codebase is in Flow which I assume has a much larger barrier for new contributors than TypeScript.

As a user of the maplibre library I'm honestly happy with just the @types provided by definitely typed. It contains everything I need to use is in my TypeScript projects.

Having that said, I do see a lot of benifits in (eventually) moving to TypeScript for the added checks that will prevent bugs down the road. With typescript, we would also no longer need to maintain a seperate @types package.

While playing with the ts code, I already found one spot where types where wrong. A method was expecting a PointLike parameter, which can be a Point object (with lng, lat attruibutes) or an array of lng lat values. In that method however the code assumes it was passed an object. With typescript you can easily spot and fix bugs like this.

It's hard to put an estimate on the effort to fix all typing errors. I might work on the conversion somewhere next week for a day, just to see how far I can get in 8 hours, to better gauge what the required effort would be.

I tend to agree with @easbar that the most important goal for now should be long term maintenance of Mapbox 1.13. Migrating to TypeScript would be great, but it's not all that important right now.

I am also wondering: Will switching to TypeScript make it any harder for users to migrate from mapbox 1.13 to maplibre? This would be a huge point against this in my opinion.
It would possibly be an issue for Flow users. Unless there's a way to generate Flow types from TypeScript definition files...

[msbarry]

I agree minimal changes are good for the 1.13 compatible version, this is more of a discussion for 2.x+ if and where there is active development on performance improvements and features.

I'm starting to appreciate the "stay with flow" argument a bit more... It looks like flow was originally added in b1e23e6 in November, 2016. Since then it appears typescript has taken off and flow has not (https://www.npmtrends.com/flow-bin-vs-typescript) but I can see how flow might lower the perceived barrier to entry for js devs compared to typescript. A javascript developer who encounters types in a js project can get their footing pretty easily, but if they encounter almost exactly the same code in a typescript project they might be turned off to contributing if they don't "know typescript". And as large as the typescript community is compared to flow, I doubt it will catch up with the size of the javascript community https://trends.google.com/trends/explore?date=today%205-y&q=typescript,javascript

Curious to hear more perspectives typescript vs. flow barrier to entry for developers who work mostly in javascript?

[zakjan]

I'm a big fan of TS on large private projects, but I agree about the perceived barrier for open-source libraries. Even if TS source code is the same as JS, a different file extension can discourage potential contributors.

Let me point to another alternative - TS JSDoc comments in JS files. TS compiler used only for type checking, and it is optional, contributors who are going to do only a small change can skip it. Bundle built with any JS compiler.

I started using them in my recently published libraries. Although the syntax is more verbose than Flow or native TS, I'm pretty happy with it, because of its universal compatibility. The most significant drawback for me is missing non-null assertion operator, type assertion needs to be written explicitly.

[HarelM]

I write in both typescript and javascript. I think we should give javascript developers a bit more credit here. Typescript is not that different and a file extension shouldn't hold someone from contributing, much like the fact I don't know flow and still wanted to contribute to this library and I sent a PR...
I agree with @folke reasoning, I constantly find bugs that way, bugs that are extremely hard to find without typescript.
I also think this should be decided early on since the migration is a lot of work and PRs may break if submitted before the migration.
I agree the main focus now should be 1.13 but looking ahead is very important too.
Bottom line, I vote in favor and would like to help if needed.

[HarelM]

Also, I recommend as an initial solution to simply add the typing file/s to this repo instead of pushing them to definitely typed and reference them in the package.json file of the npm package.
This is a very light change that shouldn't break any thing to start with.
This way you have better control over what's in here and if you change an API call you can change the definitions file too on the same branch and make sure they are aligned here instead of two different repositories and different versioning...

[curran]

What exactly are the benefits? What's the downside of not doing it?

I personally don't see any benefits at all, given the codebase already uses Flow.

how such a migration shall be justified

Agree, I don't see a strong case.

The current codebase is in Flow which I assume has a much larger barrier for new contributors than TypeScript.

Umm I don't think Flow poses any more barrier to entry than TypeScript, for JavaScript developers that have used neither in the past. Why would it?

added checks that will prevent bugs down the road

What added checks would TypeScript provide that Flow does not?

As a user of the maplibre library I'm honestly happy with just the @types provided by definitely typed. It contains everything I need to use is in my TypeScript projects.

Right?! Why bother porting the internals?

I am of the opinion that this whole premise of "let's port this massive codebase from Flow to TypeScript ... because TypeScript is better than Flow" idea is a non-issue and we, as the community around MapLibre, should not waste any more time on it.

The project has much bigger things to focus on, like for example standing up a basic Web site, and providing working reference examples, so that there is a more clear path forward for people to start using the library.

[zakjan]

I am of the opinion that this whole premise of "let's port this massive codebase from Flow to TypeScript ... because TypeScript is better than Flow" idea is a non-issue and we, as the community around MapLibre, should not waste any more time on it.

Good point. It is probably not worth the effort now. Other issues have more priority.

[wysisoft]

This would be awesome. I use javascript and typescript almost daily and for anything I work on for production I prefer to use typescript. I've caught so many my bugs in code before releasing because of its type safeness. This would make changes like getting feature state working with string id's a lot easier.

It is also possible to compile typescript apps I to web assembly which may be useful in the future.

@rbrundritt Can you reference which tool is able to comprehensively convert typescript to webassembly?

[HarelM]

This is an interesting tutorial worth exploring:
https://skovhus.github.io/blog/flow-to-typescript-migration/

[HarelM]

@folke can you please mark the npm @types library as deprecated? I've merged the typings into this repo.

As a side note, I get the feeling that each contributor has the passion to drive his things forward, we can't assume that someone that have the passion to drive the typescript effort will also drive a "reduce bundle size" effort - I think that if we play these out well will be able to harness the full potential of the community.
We should also try and follow the main trends in the market to make sure things are not getting deprecated or stale.
The following is a very good example of the trends - with good trends comes good tooling, more article, more SO questions and in general a better development experience.
https://www.npmtrends.com/flow-bin-vs-typescript

I have personally went over all the open issues and open pull requests last night to understand the impact of this change.
I believe there are just a few pull requests (mainly #165, #185, #88) that are large and we should hold until they are merged or closed to continue with this.
If someone is willing to push this forward, I'll be willing to help and back it up by helping out with the migration itself, code review etc.

[zakjan]

Re trends, recent Flow blogpost effectively admitted that Flow focuses mainly on Facebook usecases https://medium.com/flow-type/clarity-on-flows-direction-and-open-source-engagement-e721a4eb4d8b

[folke]

@HarelM happy to do that, but how? :)

[HarelM]

Didn't you created the npm package? or did you simply added a folder to the typescript repo?
If the last one, than removing this folder might change the status of this package to be deprecated, I'm not sure. It's probably worth trying to reach out to the definitely typed guys...?

[folke]

@HarelM made a PR to remove the typings and add a deprecation as of version 1.14.0

[HarelM]

Thanks!! Appreciate it.

[HarelM]

I'm starting to push the forward.
For anyone that would like to help with the effort I have created a branch for it, pull requests to this branch will allow us to push this forward.
See #209.
I'll also post on slack.
Coordination of effort will be done on the linked PR.
Thanks! and good luck to us all.

[HarelM]

Merged.