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.Collection !== Meteor.Collection.prototype.constructor #6

Closed
vsivsi opened this issue Jul 13, 2015 · 2 comments
Closed

Meteor.Collection !== Meteor.Collection.prototype.constructor #6

vsivsi opened this issue Jul 13, 2015 · 2 comments

Comments

@vsivsi
Copy link

vsivsi commented Jul 13, 2015

After loading this package, the Meteor.Collection object is left halfway between the old state and the extended state. Any package that extends the patched Meteor.Collection using prototype inheritance, such as using Coffeescript class / extends will fail to use the monkey-patched constructor.

Here's the issue where I discovered this: vsivsi/meteor-file-sample-app#2 (comment)

I'm not embarrassed to say that I'm pretty opinionated about this, and I'm going to add more sanity checks and throws to my packages to defend against other packages that misbehave when replacing Meteor provided standard system calls.

IMO, the safe and sane way to extend Mongo.Collection is via a mechanism that supports, e.g.:

extendedCollection = collectionExtend(Mongo.Collection, methods);
collection = new extendedCollection('foo');
extraExtendedCollection = collectionExtend(extendedCollection, methods);
collection2 = new extraExtendedCollection('bar');

If the app developer wants the "convenience" of setting Mongo.Collection = extraExtendedCollection then let that be their choice.

Digital-Thor pushed a commit to Digital-Thor/Adding-Sortable-to-Meteor-File-Collection that referenced this issue Jul 13, 2015
@rclai
Copy link
Collaborator

rclai commented Jul 13, 2015

I totally get your pain. I'm not a big fan of this monkey-patching business, which is one of the reasons that I wanted to create this package, reluctantly, so that the blame can get pointed to one destination, instead of many, until a more suitable solution is available.

There is a Meteor issue pending to be addressed for a better solution of extending Mongo.Collection.

I agree with the implementation that you suggest there, but that would result in a Meteor-community-sized shockwave of doom. There are numerous packages that depend on dburles:mongo-collection-instances, which depends on this. The only reasonable thing I can do is patch up this here, by adding ns.Collection.prototype.constructor = ns.Collection after it.

Aside from this, if this issue stems from Coffeescript using class.prototype.constructor as __super__, does that also mean that @matb33's matb33:collection-hooks also faces this issue in this section of code?

This was referenced Jul 13, 2015
@rclai
Copy link
Collaborator

rclai commented Jul 14, 2015

I've created a PR. Let me know if this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants