-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat: Add TypeScript definitions #9693
base: alpha
Are you sure you want to change the base?
Conversation
🚀 Thanks for opening this pull request! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #9693 +/- ##
==========================================
- Coverage 93.15% 92.57% -0.59%
==========================================
Files 187 187
Lines 15066 15079 +13
Branches 0 174 +174
==========================================
- Hits 14035 13959 -76
- Misses 1031 1104 +73
- Partials 0 16 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@dplewis use of |
@yog27ray i know but this is a foundational PR. Feel free to change it after we merge |
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.
Do the type files need to be manually generated calling npm run build:types
and then be committed as part of a PR? Would it be better to generate the type files as part of the release workflow and commit them? Or build the types in the CI and make sure there's no difference to the committed type files, to ensure types are always committed correctly, like with the options definitions.
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.
Why add this manual type file instead of waiting until it can be generated? I understand this requires contributors to manually keep this in sync with the code base and reviewers to consider this sync as a review step, right? I'm not sure we can ensure that.
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.
I tried to do Options/index.js to .ts and it broke the docs. At least we can generate something. It’s almost impossible to turn that file to ts
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.
We'll move away from the custom options logic in the future anyway. We could keep the file you added, but I'm sure that it will get out of sync over time. This could then be fixed every now and then if someone complains. A process much like DefinitelyTyped. I don't see it as a viable manual review step however. For contributors we can add it to the contribution guidelines, as least for reference.
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.
@dplewis Could you add an entry to the CONTRIBUTING guide which files are currently manually maintained and should be updated manually in case of any change of which namespaces. e.g. Parse.Cloud
? Then I think this is good to merge.
Pull Request
Issue
Generated types currently break typescript projects
Closes: #9672
Approach
Ensure index.js, ParseServer.js and LiveQuery/ParseLiveQueryServer.js are typed.