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

Add more stubs for Meteor CollectionHooks cols #21

Merged
merged 2 commits into from
Nov 23, 2017

Conversation

zeroasterisk
Copy link
Contributor

@zeroasterisk zeroasterisk commented Sep 21, 2017

CollectionHooks builds Collections with a hijacked constructor.

They "take over" most collection methods,
but StubCollections will stub them... no problem (before this commit).

Though Collectionhooks would make a direct object and allow access to
the "pre-hooked" (normal) methods for the Collection.

Until this commit, that meant that all "Stubbed" collections
worked as a "stub" for normal methods like MyCol.update(),
but would use their "non-stubbed/real" methods when used like
MyCol.direct.update()

Now we are "stubbing" both set's of methods.

TODO: It would be really nice if we could make Collectionhooks still run
it's functionality on the stubbed collection :/ -- I have not been able
to figure out how to accomplish this yet.

re #11 (again)

CollectionHooks builds Collections with a hijacked constructor.

They "take over" most collection methods,
but StubCollections will stub them... no problem (before this commit).

Though Collectionhooks would make a `direct` object and allow access to
the "pre-hooked" (normal) methods for the Collection.

Until this commit, that meant that all "Stubbed" collections
worked as a "stub" for normal methods like `MyCol.update()`,
but would use their "non-stubbed/real" methods when used like
`MyCol.direct.update()`

Now we are "stubbing" both set's of methods.

TODO: It would be really nice if we could make Collectionhooks still run
it's functionality on the stubbed collection :/ -- I have not been able
to figure out how to accomplish this yet.
@zeroasterisk
Copy link
Contributor Author

Any feedback on this?

@hwillson
Copy link
Owner

Wow, sorry for the large delay on this @zeroasterisk - I'll definitely get this reviewed shortly. Thanks!

@hwillson
Copy link
Owner

Thanks for submitting this @zeroasterisk - this all makes sense, and looks good. I'm prepping a new release for tomorrow, and will get this merged in shortly. Thanks again!

@hwillson hwillson merged commit 2f9b17b into hwillson:master Nov 23, 2017
@hwillson hwillson mentioned this pull request Nov 29, 2017
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.

None yet

2 participants