-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
TypeScript infrastructure #2206
Conversation
@datitisev perhaps we should have this PR be just the setup of Typescript infrastructure, and follow up with the actual typescript changes in a second PR? And merge the first one in now? Also, agree about putting in the shims 👍 |
@askvortsov1 Ok, done. EDIT: I won't put the shims here. They aren't necessary (though they do help with typings, but more when typing Mithril stuff so lets wait for mithril v2 there maybe? idk) |
9ca2d27
to
cdee757
Compare
cdee757
to
0a72f6b
Compare
Thanks, looks good! Can we undraft this to get it merged? Just one question: what's the purpose of |
I... don't know what that does. I copied it over from my Mithril branch. Looking at the babel docs for that package did not help in understanding. I think it has something to do with |
Yeah, those allow you to import scripts at runtime, I don't think that's needed for TypeScript at all. Please remove that, then we can merge. 🤗 |
0e20978
to
14cc582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! TS-ing "comon" and "admin" should now be safe. (One file in "common" is changed in #2161, but we will manage.)
Will probably want to include some shims
Old example shims in WIP rewrite branch
https://github.com/flarum/core/blob/4c8af1cdf876b9bd3770f76c33eb6c9ac80608f0/js/webpack-flarum-shims.d.ts#L7-L17
https://github.com/flarum/core/blob/4c8af1cdf876b9bd3770f76c33eb6c9ac80608f0/js/shims.d.ts#L3-L7