-
-
Notifications
You must be signed in to change notification settings - Fork 834
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
Convert core JS to Typescript #3533
Comments
Hey maintainers! |
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 https://github.com/flarum/core/blob/9cb9097b24e0345816f2f8ef22789ab804e796f6/js/tsconfig.json#L2 |
@nina-py Yes, we use
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 |
Maybe that might also help with the ESDoc generation issue? Although I doubt it |
@askvortsov1 Don't think so - that's a separate config file anyway. |
Thanks @datitisev, a PR is on the way.
According to the docs (see quote below), all that needs to be done is removing the
|
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! |
Hi everyone! I'm in the same @Niveditha-K17 situation, i want to contribute, it would be a fantastic way to improve my Typescript skills. Still need PRs? |
Yeah, there's still plenty to convert. See for instance this path: https://github.com/flarum/framework/tree/2.x/framework/core/js/src/forum/components |
Ok, thank you. Can I take files freely or do I have to warn/agree with someone? I read that there was talk of agreeing on Discord but not being on the server I don't know |
Hey everyone! Is there anything left to convert?? would love to help |
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
The text was updated successfully, but these errors were encountered: