Feature suggestion: Track registered Mongo collection instances.

[JorgenVatle]

The ability to retrieve a collection by name should ease with some edge cases where package authors rely on shims for the Mongo.Collection module in order to add functionality. As far as I'm aware, there aren't really any good ways of retrieving collections by name, you need to import them explicitly, or patch directly Mongo.Collection to add this functionality.

cultofcoders:redis-oplog is a good example of where that doesn't always work out well. Collections registered by compiler/build plugin packages appear to be registered before standard feature packages. This leads to the oplog package having no real way to reliably access those collection instances.

Just opening this as a draft PR so we can have a quick discussion whether this would be a worthwhile addition. 👍

[CLAassistant]

All committers have signed the CLA.

[nachocodoner]

I've made similar changes in past projects and believe it's good to integrate them into the core.

Anyone has any inconvenience here?

[harryadel]

How does this affect https://github.com/Meteor-Community-Packages/mongo-collection-instances?

[JorgenVatle]

The proposed changes shouldn't have any impact on mongo-collection-instances. The collection map is added under Mongo.Collections not Mongo.Collection.

Though, seeing this package's implementation, it would be nice to have a simple .get() method on Mongo.Collection, then hiding away the Collections map with an underscore or something to indicate it's something that shouldn't be accessed directly. In this case, mongo-collection-instances should still function correctly, it would just override the .get() method provided by Meteor. The collection map would still work and be accessible to other packages as it is tracked in the Mongo.Collection constructor.

[JorgenVatle]

A .get() method directly on Mongo.Collection would probably a nicer way to access registered collections instead of exposing a plain Map instance.

I'd be happy to update the PR accordingly next week and add some documentation for it if it sounds good to you. 👍

[harryadel]

Thanks for the explanation Jorgen, I'm no part of the core team but if @nachocodoner & @leonardoventurini, maybe even @radekmie since he has done many mongo performance improvements then I think it'd be a good idea to act on. Thank you for your efforts.

[radekmie]

I think it's fine to add such a global registry, but I definitely would not make it a public API. It can be one of those "documented internals" instead, where Meteor would try to but wouldn't have to keep backward compatibility.

Also, it's now technically possible to get all collection names through drivers but not their instances, so I'd call it something like Mongo._collectionInstances.

[leonardoventurini]

This might come in handy to prevent circular imports. I've used mongoose in the past quite a bit for raw Node.js projects, and it allows us to get the model by simply calling model('name'), this helps when we reference other collections inside hooks, instead of importing them there directly, which as the collections are sometimes exported all at once, could cause a chicken and egg dilemma. With that, I believe having this ability in Meteor is a great add, but I would vouch for making this a public API, perhaps making it more ergonomic even like collection('name').

[JorgenVatle]

Haven't had the chance to follow up the proposal just yet. Got some time on Monday and/or Tuesday and will update the PR from there. 👍