First-class types for Mapbox Draw

[brookjordan]

Now that we’ve got first-class types for MapBox GL.js, is it time we have the same for draw?

I’m currently working on types here: https://github.com/brookjordan/mapbox-gl-draw/tree/chore/convert-to-typescript/bj

I was wondering if this would be a forever-patch on my side, or if it’s something we would like to have officially supported, if I can get the first version up and running?

[stepankuzmin]

Hey @brookjordan,

Yeah, it's a good time to migrate GL Draw to first-class TypeScript. I first wanted to modernize the internals (i.e., switch to modern ES), but this might be easier with first-class types.

I wonder what your process for the migration is. Do you manually rewrite the JS to TS, or do you use some automated tooling? We were using patched Stripe's codemod to convert GL JS from Flow to TS.

[brookjordan]

Manually.

There's not a lot of code, and converting files to .ts gets you a lot of the way there, and allows me to look at what's actually going on.

Like, there's a bunch of functions pretending to be classes, so if I'm looking at all of the code anyway, it's quite easy to do the conversion to classes while I'm there. A small part of the es6 update.

However, I'm leaving some things the same, like object.assign, so it's easier to review with less syntactic changes. Converting to a class keeps the code very similar, other than the wrapper. Keeping the changes small enough that people are willing to review it felt important.

I'm planning to keep tests in .js for now, as there's no real demand for those to be converted, and it allows us to test how a non .ts codebase using the code could abuse the code without a plethora of ts-disable comments.

[stepankuzmin]

I see. Thanks for the initiative, @brookjordan!

It would be great to start small with the TypeScript migration. First, we can add initial TypeScript support to the repo and ensure compatibility with both JS and TS. Then, we can create separate PRs for converting files one by one. I'll be happy to help with this process.

[brookjordan]

That would actually be a nice idea… maybe the first commit in my branch would work as that first step, attempting to parse as TypeScript without strict mode.

[cfecherolle]

I'm having some typing problems too since upgrading to mapbox-gl 3.5.2. Custom events such as:

this.map.on("draw.snap.options_changed", optionsChangedCallback);

Trigger an error:

TS2345: Argument of type "draw.snap.options_changed" is not assignable to parameter of type MapEventType

But used to be accepted by TS before the upgrade to 3.5.*

Can I safely ignore this with a // @ts-expect-error or could it cause issues?

[guillem-aistech]

Interested!