flarum
Convert core JS to Typescript
This should be done in small, maximally isolated PRs: no more than a few files updated per PR so that we can slowly review and phase these in over time. Community contributions appreciated!
For overall progress, see the GitHub Project board: https://github.com/orgs/flarum/projects/21
Hey maintainers! I would love to take this issue up, looks like an amazing learning opportunity! I'm not very experienced with Typescript, but know the basics.
That's great to hear, and we'd definitely appreciate the help! https://docs.flarum.org/contributing.html might be a good place to start for setup stuff (and reading a bit more about how flarum works). Our community forums and discord chat are also good resources if you have questions / run into issues.
Great to hear! I've joined the discord chat and I'll go through the links in a few hours and get started soon!
Is this a good issue for multiple contributors? I'd also like to work on this, and I've joined the discord so we might make sure not to double up on work.
@knoxd8256 this would be a great issue for multiple contributors, my recommendation would be to negotiate work being done via our discord chat and/or this issue so that we don't have repeat file updates.
Can I confirm that the file naming conventions for TypeScript files are .ts for general code and .tsx for anything containing JSX-like syntax? The linked PRs seem to be following these naming conventions but the unconverted JS files are all .js. The reason I'm asking is that if .tsx files are the new norm for components and the like, this line in tsconfig.json needs to be updated to include them to get rid of certain warnings, i.e. the esModuleInterop one:
https://github.com/flarum/core/blob/9cb9097b24e0345816f2f8ef22789ab804e796f6/js/tsconfig.json#L2
@nina-py Yes, we use .tsx for JSX.
this line in
tsconfig.jsonneeds to be updated to include them to get rid of certain warnings, i.e. theesModuleInteropone:
Hm, if that's the source of the warning then that's a big 🤦 for me. Feel free to PR to that - it might be enough if you just add * after *.ts, can't remember if that works or not.
Maybe that might also help with the ESDoc generation issue? Although I doubt it
Thanks @datitisev, a PR is on the way.
Hm, if that's the source of the warning then that's a big facepalm for me. Feel free to PR to that - it might be enough if you just add * after *.ts, can't remember if that works or not.
According to the docs (see quote below), all that needs to be done is removing the .ts. file extension and it will automatically pick up only the converted files and not the entire codebase.
If a glob pattern doesn’t include a file extension, then only files with supported extensions are included (e.g. .ts, .tsx, and .d.ts by default, with .js and .jsx if allowJs is set to true).
https://www.typescriptlang.org/tsconfig#include
Also, there are more warnings in the files that's already been converted to TypeScript. Wouldn't it be great if these were caught automatically by a GitHub action? ESLint has a plugin for TypeScript, perhaps it could be added to the workflow?
@nina-py Resolving TS warnings is not a priority because we're using TS to help typehinting mainly, not to have perfect code (in terms of TS warnings). If you want to PR fixes for warnings tho, go ahead (unless they make the code ugly, then we may ignore it).
Thanks @datitisev, I certainly don't have an intention of making any code ugly. Either way, I need to read up on Mithril and get a bit more comfortable with the Flarum frontend code before starting on this.
Hi Guys and Girls , just join this wonderful collab , is there a specific channel for this on discord , just would like to know which files everyone might be working on so not so double do ..
I saw your message on discord @sweetrush. Internals would be the right channel 👍
Hello maintainers! I'm new to Typescript but I'm eager to learn and would love to contribute. Is there any issue I can take up?
