Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using TypeScript #118

Open
fgblomqvist opened this issue Oct 12, 2022 · 9 comments
Open

Consider using TypeScript #118

fgblomqvist opened this issue Oct 12, 2022 · 9 comments

Comments

@fgblomqvist
Copy link
Contributor

With risk of a déja vu (@peterklingelhofer knows what I'm talking about), I'd strongly recommend you guys to consider using TypeScript for this project, because it makes it so much easier for new and random contributors to get started. Working on non-typed code can be incredibly frustrating if you're not very familiar with it, and you're often left with spending large chunks of your time just understanding what objects various functions expect. Not to mention the time you save not having to debug your own code.

If nothing else, if a decision has already been made against it, it would probably make sense to document that somewhere (like in this thread), because I can guarantee you I won't be the first to bring it up 🙂

But great initiative restarting this project, much needed 🚀

@simonv3
Copy link
Collaborator

simonv3 commented Oct 12, 2022

Hey @fgblomqvist!

All three of the main devs (@peterklingelhofer , @mattburnett-repo , and myself, currently) I think are in alignment that we ultimately want to see this codebase in TypeScript. However, right now we're focusing on combining the existing JavaScript APIs (as outlined in the Active Development section of the docs and further elaborated on in the forum here and here.

A significant part of this work is learning what the API does, and to make sure that it continues doing this correctly. So there's been a lot of focus on migrating functionality, writing tests, etc.

Once we know what we've got, I think @peterklingelhofer would love to take a crack at starting a TypeScript conversion ;) (I'd personally prefer to have that happen fairly incrementally rather than all in one go, but tests will help catch any mishaps, and ultimately it's up to the person taking it on).

@fgblomqvist
Copy link
Contributor Author

Thanks for the elaborate response, sounds reasonable! Hoping to contribute some day, but figured I'd at least drop this note sooner rather than later since it might help with the migration (writing types is kinda like writing tests at the end of the day), and it solves issues like #21. But understandable if you'd rather wait!

@mattburnett-repo
Copy link
Collaborator

Hallo

Hard agree on everything mentioned by everyone already. Anything I would add to the discussion is some form of 'me too' comments, so I'll just leave it at that.

By yeah, I want to go to the TypeScript party too!

@fgblomqvist
Copy link
Contributor Author

btw is there a reason you guys are doing CJS instead of ESM? Will make the migration to TS a bit more of a pain, no matter if it happens now or in a year

@simonv3
Copy link
Collaborator

simonv3 commented Oct 13, 2022

@fgblomqvist We're combining code from a bunch of different repositories into one. This isn't a clean rewrite, so there's a lot of legacy code that none of the currently active devs actually wrote (or even might not know exists). If you think there's a better way to do something, you are welcome to propose it (especially if you're willing to help implement it, or even just give pointers on how!).

Edit: and to clarify and actually answer your question, no, there isn't a reason :)

@fgblomqvist
Copy link
Contributor Author

Ah okay, wasn't sure that was the case (copy/paste) but suspected it. Should be possible to do ESM file-by-file by changing the extension to .mjs to convert on-the-go (or for any further files you're bringing over, if you don't mind changing the imports to ESM style) 🙂

In terms of contributing, I'll see if I can try to help out with one of the open issues next week

@simonv3
Copy link
Collaborator

simonv3 commented Oct 13, 2022

If you want to make a ticket for the conversion and put in an example PR that would be greatly appreciated!

@dominictwlee
Copy link

dominictwlee commented Nov 22, 2022

https://www.typescriptlang.org/tsconfig#JavaScript_Support_6247

Gradual migrations are definitely feasible, and quite common at a few places I previously worked at with lots of legacy code.

@peterklingelhofer
Copy link
Member

Api/beam have hit feature parity with live, so definitely worth tackling this/considering this high priority. Related: #169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants