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

meteor: strong typings for mongo collections selectors and modifiers #25210

Merged
merged 3 commits into from
Apr 24, 2018
Merged

meteor: strong typings for mongo collections selectors and modifiers #25210

merged 3 commits into from
Apr 24, 2018

Conversation

andrei-markeev
Copy link
Contributor

Rationale

Personally was struggling for a long time with the lack of intellisense in Mongo selectors and modifiers. This prevents efficient refactoring of data model. Using mapped and conditional types from TypeScript 2.8 I finally managed to solve this problem, and all selectors except composite (e.g. "arr.$.prop") are now covered by intellisense. Thanks to this change I was able to spot couple of serious bugs in a production application I am developing.

Checklist

  • Use a meaningful title for the pull request. Include the name of the package modified.

  • Test the change in your own code. (Compile and run.)

  • Add or edit tests to reflect the change. (Run with npm test.)

  • Follow the advice from the readme.

  • Avoid common mistakes.

  • Run npm run lint package-name (or tsc if no tslint.json is present).

  • Provide a URL to documentation or source code which provides context for the suggested changes: https://docs.mongodb.com/manual/reference/

  • Increase the version number in the header if appropriate.

  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

@typescript-bot
Copy link
Contributor

typescript-bot commented Apr 22, 2018

@andrei-markeev Thank you for submitting this PR!

🔔 @pgrm @barbatus @fullflavedave @orefalo @dagatsoin @birkskyum @ardatan @stefanholzapfel @vangorra @mattmm3d - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@ardatan
Copy link
Contributor

ardatan commented Apr 22, 2018

Thanks for this great enhancements!
Does this update satisfy the usage?

// (obj: { attr })
Collection.find({
'obj.attr': value
})

@andrei-markeev
Copy link
Contributor Author

andrei-markeev commented Apr 22, 2018

Yes, it does, note the & Dictionary<any> in the definitions. But when using this syntax, obviously there won't be any good intellisense.

As you can see on the screenshots below, find that uses 'inlineLinks.objectType' passes validation alright, while still preserving correct intellisense when using authorId or other properties.

image

image

@ardatan
Copy link
Contributor

ardatan commented Apr 22, 2018

Yes, you're right, it's my fault. Sorry for that.
Thank you again for this great update!
It looks good to me!

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Apr 22, 2018
@typescript-bot
Copy link
Contributor

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@weswigham weswigham merged commit 3d6c3d2 into DefinitelyTyped:master Apr 24, 2018
@ardatan
Copy link
Contributor

ardatan commented May 9, 2018

Hello, many people are using these typings for Angular-Meteor projects, then they are facing some problems because Angular uses TypeScript 2.7.2. So is it possible to make it compatible?

@andrei-markeev
Copy link
Contributor Author

andrei-markeev commented May 10, 2018

Conditional and mapped types are supported only starting from TS 2.8... the only option to make those compatible seems to be roll back the MongoDb intellisense completely and wait until Angular upgrades to TS 2.8+...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Owner Approved A listed owner of this package signed off on the pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants