search

jonorthwash / ud-annotatrix

GNU General Public License v3.0
59 stars 49 forks source link

Start adding support for TypeScript #496

Closed keggsmurph21 closed 1 year ago

keggsmurph21 commented 1 year ago

TypeScript, if you're not familiar, is basically JavaScript with types. My thinking is that having types will make refactoring / adding new features much easier, in the long run.

This PR takes some baby steps towards converting the project into TypeScript (luckily, this can be done incrementally). At the moment, the only thing that has types in notatrix/src/utils/ (just 5 files).

If you're just working on client/ or server/, this change should be totally transparent.

If you're working on notatrix/, then you'll need to make sure you re-compile the TypeScript before rebuilding the server bundle. In other words, if you're used to

$ npm run build

then you'll now need to

$ cd notatrix/
$ npx tsc
$ cd ../
$ npm run build

Eventually, I'd like to remove this^ restriction, but it'd be a lot more boilerplate I think...

If you want to take a look, I'd recommend reviewing commit-by-commit!

keggsmurph21 commented 1 year ago

@jonorthwash @ftyers @Zensho do you think this would be worthwhile?

jonorthwash commented 1 year ago

I think this is a good idea, from the little I know about TypeScript. I'm not able to review the commit in any competent way, so if you think it's good we can probably just merge it.

jonorthwash commented 1 year ago

Are the new build commands documented somewhere?

Zensho commented 1 year ago

I think this looks great and while it might not help with my current backend rewriting that much, it would make any further adjustments and fixes easier once we do a complete typescript transformation of the current code.

keggsmurph21 commented 1 year ago

Are the new build commands documented somewhere?

Haha, great question! I added some documentation in https://github.com/jonorthwash/ud-annotatrix/pull/496/commits/a1da42a44e4ba9ad23606c9ebc29ba857a84f367. Let me know if there's anything else you want me to include.

I'm not able to review the commit in any competent way, so if you think it's good we can probably just merge it.

Ok, sounds good! I just wanted to make sure you had a chance to veto before I hit "merge" :grin: