fgrandel / meteor-versioning

Meteor redo/undo, improved conflict resolution and basic transactions support.
Other
22 stars 2 forks source link

Running the example give an error: #2

Open ferakiii opened 11 years ago

ferakiii commented 11 years ago

Unless I'm doing something wrong in setup, I've just copied pasted the example code in main.js for a freshly created project.

It gives the following error:

/Users/ferakiii/Dev/Standbench/versioning/.meteor/local/build/server/server.js:337 }).run(); ^ Error: use "new" to construct a Meteor.Collection at new Meteor.Collection (app/packages/mongo-livedata/collection.js:7:11) at new Collection (app/packages/versioning/versioned-collection.js:26:21) at app/versioning.js:4:13 at /Users/ferakiii/Dev/Standbench/versioning/.meteor/local/build/server/server.js:298:12 at Array.forEach (native) at Function..each..forEach (/Users/ferakiii/.meteor/tools/cc18dfef9e/lib/node_modules/underscore/underscore.js:78:11) at run (/Users/ferakiii/Dev/Standbench/versioning/.meteor/local/build/server/server.js:239:7)

fgrandel commented 11 years ago

Hi ferakiii!

Sorry to hear that the example no longer works. This is probably an incompatibility with recent versions of Meteor. As I'm in a different project now I didn't have the time to upgrade the package to the latest versions. I don't know when I'll be able to fix this. If you like I can help you debugging the problem, though. AFAICS there's no way to retract a package from Meteorite. Otherwise that should probably be done.

ferakiii commented 11 years ago

I'll take a look and see if I can figure out whats going on, as this seems like something that would be great in my current project. Did you get the historical viewing working for this package?

fgrandel commented 11 years ago

I'll take a look and see if I can figure out whats going on

That'd be great.

Did you get the historical viewing working for this package?

No, I didn't have a use case. But I don't think it would be difficult to implement that from the already existing version history. If you like I can assist you in implementing that (answering your questions, etc.).

ferakiii commented 11 years ago

So this assertion is failing in the meteor base collection: collection.js

Meteor.Collection = function (name, options) { var self = this; if (! (self instanceof Meteor.Collection)) throw new Error('use "new" to construct a Meteor.Collection');

I'm guessing that since your package redefines Meteor.Collection, it is checking itself versus that. I'm not sure what to change to get around this, as I'm guessing not redefining Meteor.Collection (say making another class altogether, like VersionedCollection) will fail, as other references inside the Meteor core will need to refer to the new versioned collection...

fgrandel commented 11 years ago

I'll have a look later on today and I'll come back to you.

On 06/19/2013 02:46 PM, ferakiii wrote:

So this assertion is failing in the meteor base collection: collection.js

Meteor.Collection = function (name, options) { var self = this; if (! (self instanceof Meteor.Collection)) throw new Error('use "new" to construct a Meteor.Collection');

I'm guessing that since your package redefines Meteor.Collection, it is checking itself versus that. I'm not sure what to change to get around this, as I'm guessing not redefining Meteor.Collection (say making another class altogether, like VersionedCollection) will fail, as other references inside the Meteor core will need to refer to the new versioned collection...


Reply to this email directly or view it on GitHub: https://github.com/jerico-dev/meteor-versioning/issues/2#issuecomment-19700816

fgrandel commented 11 years ago

Sorry, I got too much work currently. But I've not forgotten this issue... I'll look into it.

ferakiii commented 11 years ago

Hey, I got it running, by stopping the versioned-collection from overwriting the base Meteor.Collection, and instead renaming it to Meteor.VersionedCollection. I'm not sure what this will effect as it goes on, but it seem to let the example run, and works with another example project I'm testing this package out with. One issue it has raised though is wether calls to the server via Meteor.Call can get added to the client undo stack. They seem to be added to the server undo stack, and therefore cannot be undone from the client (except by creating a Meteor.method that does the undo). This however lets you undo anyones calls, and not just your own. Can this package only undo simple edits to the collections on the client side?

fgrandel commented 11 years ago

Hi, I reproduced the error today. It is due to this change: https://github.com/meteor/meteor/pull/995/files

Renaming to VersionedCollection will probably work if you also rename all occurrences of OriginalCollection back to Meteor.Collection (which you probably did). Of course it would be nicer if we could continue to patch Meteor.Collection but it will no longer work the way it did so far.

Can you provide a patch (code & documentation) that fixes the issue? If you like you could completely take over the package. I don't have time to look after it any more. I'd already have retracted it if meteorite/atmosphere provided such a possibility. If you cannot provide a patch then I'll put a warning in the README that the package is no longer being maintained.

Wrt your question: Calls to Meteor.call are not logged directly to the undo stack. In fact I'd avoid them altogether as they'll just make your application's timing/synchronization unnecessarily complex. The transaction model completely replaces remote method calls in my applications. Can you describe a use case that requires a remote method call that could not be replaced by (or: use) transactions? I suggest that you open up a separate issue if you like to further discuss this topic.