Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Why doesn't MongooseArray pop or shift produce a _delta()? #821

Closed
timoxley opened this Issue Apr 9, 2012 · 8 comments

Comments

Projects
None yet
2 participants
Contributor

timoxley commented Apr 9, 2012

If i have an array in my schema

var assert = require('assert')
var mongoose = require('mongoose')
var Schema = mongoose.Schema
mongoose.connect('mongodb://localhost/mongoosearraysettertest ')

var DummySchema = new Schema({
  name: String,
  items: [Number]
})

DummySchema.pre('save', function(next) {
  var delta = this._delta()
  console.log('pre save delta', delta) // prints delta if available
  next()
})

var Dummy = mongoose.model('Dummy', DummySchema)

Dummy.create({
  items: [1,2,3,4,5]
}, function(err, d) {
  assert.ifError(err)
  d.items.pop() // does not create delta??
  d.save(function(err, d) {
    assert.ifError(err)
  })
})

The presave produces "pre save delta undefined"

Same happens with shift but not push, $shift or $pop. Why is that? Seems inconsistent. If this is a bug, I'd be willing to fix it.

Also, I''m analysing _delta() to know what elements are added/removed from an array, is this safe (considering it's a _private method)? Is there another way to get the delta?

Collaborator

aheckmann commented Apr 11, 2012

yeah its a bug. please submit a patch.

Contributor

timoxley commented Apr 26, 2012

Moved onto a different project temporarily, patch will come… but keen to know if you have an answer to the second part of the question, @aheckmann :

Also, I''m analysing _delta() to know what elements are added/removed from an array, is this safe (considering it's a _private method)? Is there another way to get the delta?

Thanks man

Collaborator

aheckmann commented Apr 27, 2012

its safe and there is no better way unless you hook the markModified method and watch the paths but even then you don't see the values so its pretty pointless.

Contributor

timoxley commented Apr 27, 2012

Awesome, thanks heaps.

Contributor

timoxley commented May 1, 2012

@aheckmann I don't actually understand what the $ prefix is supposed to mean vs no $, other than to provide consistency with the mongodb operations. In particular, I don't understand why we have:

MongooseArray.prototype.$push =
MongooseArray.prototype.push = function () {

but not:

MongooseArray.prototype.pop =
MongooseArray.prototype.$pop = function () {
Collaborator

aheckmann commented May 1, 2012

It was to be consistent with mongodb commands but that is going away in master. One of the 3.x goals is to remove all the aliasing and reduce the codebase bloat.

On Apr 30, 2012, at 10:25 PM, Tim Oxleyreply@reply.github.com wrote:

@aheckmann I don't actually understand what the $ prefix is supposed to mean vs no $, other than to provide consistency with the mongodb operations. In particular, I don't understand why we have:

MongooseArray.prototype.$push =
MongooseArray.prototype.push = function () {

but not:

MongooseArray.prototype.pop =
MongooseArray.prototype.$pop = function () {

Reply to this email directly or view it on GitHub:
LearnBoost#821 (comment)

Collaborator

aheckmann commented May 2, 2012

I think this is fixed now right?

Contributor

timoxley commented May 2, 2012

Yes, the issue was that _registerAtomic wasn't being called for any regular array operations other than push, the 'new' aliases force any regular push, pop, shift, unshift interaction with the array to go via mongoose.

@timoxley timoxley closed this May 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment