search

Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

Update for 0.9.1 #57

Closed subhog closed 10 years ago

subhog commented 10 years ago

Updated API and dependencies for Meteor 0.9.1 version.

mizzao commented 10 years ago

@matb33 tried to make this backward-compatible, so I imagine we'll probably stay on the old names for now for simplicity.

subhog commented 10 years ago

The problem is that using the old names causes problems between packages. Among others, the current version is incompatible with aldeed:collection2. I'm investigating whether there's another solution, but updating this package does solve the problem.

Perhaps detecting API version and varying the used object would be acceptable?

subhog commented 10 years ago

It can be a pretty simple addition:

Mongo = Mongo || Meteor;
Tracker = Tracker || Deps;

Update: apparently, it cannot, the app still crashes when this trick is used. But creating collections with conditional works:

Docs = Mongo ? new Mongo.Collection('docs') : new Meteor.Collection('docs');
subhog commented 10 years ago

Alternative solution: https://github.com/meteor/meteor/pull/2527

subhog commented 10 years ago

Added pieces for backward compatibility. Basically, collections are created via (Mongo ? Mongo : Meteor).Collection. Some tweaks may still be necessary for package.js.

matb33 commented 10 years ago

Thanks @subhog. There were some tweaks to make but the general approach of trying for Mongo/Tracker and falling back to Meteor/Deps respectively was preserved ((Mongo ? Mongo : Meteor) will throw an error if Mongo is not defined, so you can either do a typeof Mongo !== undefined, or look for Mongo as a property nested inside the Package global, which is the route I took). Thanks for contributing!

mizzao commented 10 years ago

@matb33 may want to create and push the v0.7.4 tag, perhaps?

mizzao commented 10 years ago

@matb33 whatever you did in this version breaks new Meteor.Collection("something"), as I tried to report in meteor/meteor#2549 and then realized it was my own problem.

I liked the old one better...?

matb33 commented 10 years ago

I've attempted to support both new Meteor.Collection and new Mongo.Collection, but had to resort to dynamically reassigning the prototype... I throw up a console.warn when this happens.

Also I'm lost as to how to run test-packages against 0.8.3 etc, so I'm not even sure that the backwards compat stuff even works as intended... I suppose if someone tries to use it with an older version of Meteor, they can let me know if it breaks!

mizzao commented 10 years ago

Does meteor test-packages ./ --release 0.8.3 not do it? (Or as part of an app, meteor test-packages matb33:collection-hooks --release 0.8.3 - it doesn't matter that there is a colon if the package is installed locally.)

I don't think you actually published 0.7.4 to the server yesterday. (http://atmospherejs.com/matb33/collection-hooks)

So what we had prior to these fixes worked fine with Meteor.Collection. Was the problem that it didn't like Mongo.Collection?

matb33 commented 10 years ago

@mizzao thanks, tried meteor test-packages ./ --release 0.8.3 but that just dumps "Error: Invalid package name: matb33:collection-hooks" on me. Probably the folder name...

I didn't publish 0.7.4, and since I did the fix for supporting Meteor.Collection, I figured I'd skip publishing it and go straight to 0.7.5.

Well, apparently there were interoperability issues with other packages if we kept on using the deprecated Meteor.Collection, so that's why I tried to use Mongo.Collection primarily, and falling back to Meteor.Collection. But as @subhog found, it's trickier than it seems...

Sewdn commented 9 years ago

Using 0.7.6, with >Meteor@0.9.4 I still receive the Error: use "new" to construct a Mongo.Collection:

Error: use "new" to construct a Mongo.Collection
W20141119-10:57:32.999(1)? (STDERR)     at new Mongo.Collection (packages/mongo/collection.js:29)
W20141119-10:57:32.999(1)? (STDERR)     at ns.Collection (packages/matb33:collection-hooks/collection-hooks.js:190)

In my case Mongo.Collection should be available. Does anyone else have the same issue?

Sewdn commented 9 years ago

Ok, resolved for me: When you are defining a new Mongo.Collection somewhere in your code, without declaring a dependency on the mongo package, the build process throws this error, making it look like it is an issue in colleciton-hooks. You don't get any clue that you have a missing dependency somewhere.