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

Extending further #240

Merged

Conversation

aramk
Copy link
Contributor

@aramk aramk commented Apr 4, 2018

I resolved an issue I was facing with using https://github.com/Sewdn/meteor-collection-behaviours. This library uses the wrapCollection() function a second time, which in Meteor 1.6.1 leads to discarding the extended version meteor-collection-hooks applies. My changes ensure that when creating a collection, the version in meteor-collection-behaviours will call the version in meteor-collection-hooks as well.

@droka
Copy link

droka commented Jul 10, 2019

OK. So this PR is fixing two things.
The first is not related to Meteor version 1.6.1, it is related to lai:collection-extensions trying to use same variable name for storing the old constructor as collection-hooks, so they conflict with each other. Anyone using both packages (collection-extensions and collection-hooks) will have the problem of hooks not running because of this.
You fixed this by using a different variable in collection_hooks now. This solves the issue. (Another solution would have been using lai:collection-extensions for wrapping, as was the plan for quite some time, but it did not happen for some reason, so there is now an exact duplicate of the wrapping code in the two packages, using the same variable names -> causing the problem)
The other issue that this PR is solving is to be able to call Mongo.Constructor without "new", which is a Meteor 1.6.1 related thing indeed. I'm not sure if later meteor patches fixed this already?
But the first problem needs fixing badly for many of us. Can that be pulled into master? Otherwise no one can use collection-hooks together with any other package using collection-extensions at the same time.

Copy link
Member

@StorytellerCZ StorytellerCZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested and it indeed fixes the 1.6.1 problem. Will merge asap. Thanks a lot!

@StorytellerCZ StorytellerCZ merged commit 2be77d0 into Meteor-Community-Packages:master Oct 8, 2019
@aramk aramk deleted the meteor-1.6.1-aramk branch October 8, 2019 20:37
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

Successfully merging this pull request may close these issues.

3 participants