quagga2 icon indicating copy to clipboard operation
quagga2 copied to clipboard

update to typescript

Open ericblade opened this issue 4 years ago • 11 comments

https://github.com/serratus/quaggaJS/pull/385

I'm totally interested in doing TypeScript. I haven't done anything with TS before, so not exactly my forte, but I'm very interested in picking it up. There's been a lot of interest by others in the past, and with the pull request referenced above, it'd be a fantastic point to get started with.

ericblade avatar Aug 13 '19 22:08 ericblade

I have a bit of TypeScript experience so if you get stuck on something or need a second opinion about something, I'm available to try and help out.

TomasHubelbauer avatar Aug 21 '19 08:08 TomasHubelbauer

There is a fork which includes a TS rewrite: https://github.com/Domratchev/quaggaJS/commits/master

I have not looked at it closely, maybe it was done in automated fashion, maybe it was done by hand nicely. If this issue covers rewriting this fork in TypeScript, the linked fork might be a good resource to have on hand.

TomasHubelbauer avatar Aug 21 '19 11:08 TomasHubelbauer

yep, that should be same as the one i linked to (though they might've done more work to it since). unfortunately, it was a huge amount of changes in one commit, but i've already done a pretty fair amount of the changes that weren't related to typescript in that commit already, either separately, or using that commit as a guideline (for upgrading babel, for example)

ericblade avatar Aug 21 '19 16:08 ericblade

I started to mess with TS in the attempt at integrating external reader code.. but I haven't really touched any of it in the main repo. This is a pretty sizeable project, and I really don't want to convert the entire thing all in one shot, especially with somewhat questionable test coverage. I'm pretty sure that it should be possible to configure everything in such a way that it can be handled on a small commit basis, rather than a giant commit that changes everything all at once.

ericblade avatar Dec 09 '19 19:12 ericblade

@ericblade I think it's hard to do it only in small peaces. Wouldn'T it be the best to start on a new Repo, for example QuaggaTS wich would be a successor, and we use the fork above as a start?

jogibear9988 avatar Dec 10 '19 18:12 jogibear9988

There are many problems with doing the entire thing at once, one of which I'm really highly concerned with, is that until the test suite is considerably better than it is right now, it's quite difficult to deal with a large changeset if it does end up breaking something that isn't explicitly tested.

We've already seen situations where I've made very small seemingly innocuous changes, that didn't break anything in the test suite, but totally broke a lot of functionality.

From doing some Googling, it seems some people suggest starting with changing your main entry point into typescript first, then converting the rest of it as it goes. However, since we already have a type definition file for the main entry point (quagga.js, quagga.d.ts), I think it might be a better idea to explicitly write type definitions for the other classes first -- starting with one that is exported and used externally, image_wrapper.js .

I've filed #81 for that purpose explicitly. The external QR code reader that I've put together is my first typescript project (though it is extremely simple), and could be a good roadmap to converting without breaking. Perhaps after types are written for the other utility classes, converting the readers might be a good second step?

ericblade avatar Dec 11 '19 14:12 ericblade

types added for ImageWrapper and SubImage, works in VScode with quagga2-reader-qr

ericblade avatar Dec 12 '19 20:12 ericblade

@jogibear9988 i've just added a few commits that should get this pretty close to running with typescript. https://github.com/ericblade/quagga2/pull/87

I've got an issue right now where i can't get the tests to run with the awesome-typescript-loader, however, and that does present a problem.

If you have any ideas I'd love to hear :-)

ericblade avatar Dec 15 '19 13:12 ericblade

FWIW, Domratchev's commits continue using babel-loader, whereas I switched it to use awesome-typescript-loader . . . which was incredibly painless for the main source code, but there's definitely something odd about the karma configs, which might need to be unwound some to deal with it, or at least have a better understanding than 'simply replace babel-loader with awesome-typescript-loader'.

ericblade avatar Dec 16 '19 18:12 ericblade

typescript branch is now using babel-loader with @babel/typescript plugin, per https://iamturns.com/typescript-babel/

doesn't support real-time type checking directly (i think there's a command in that blog post that says how to get it), but with the support of real time handling inside editors, i don't think it's terrible to not have it. i'll see how it goes with typing up a few files.

ericblade avatar Dec 17 '19 08:12 ericblade

wow, that was quite a task, but the typescript branch now has running typescript support for both main code and tests. Probably a good time to have a good look at @Domratchev repo and see how that aligns with what is here now.

ericblade avatar Dec 17 '19 11:12 ericblade

closing this out now, a large portion of the codebase has become typescript now, and another large portion i'm working on right now. I'll probably make separate tasks for pieces that remain after today.

ericblade avatar Sep 05 '22 07:09 ericblade