key value as mapper.idAttribute property

[JovanT]

as return from _find and _findAll we have result array of objects result of datasnapshot.val() and key of the result is not part of the result set.

I suggest that we create mapper.idAttribute property on the result set objects array.

_findAll will look like this:

  _findAll (mapper, query, opts) {
    query || (query = {})
    opts || (opts = {})

    const collectionRef = this.getRef(mapper, opts)

    return collectionRef.once('value').then((dataSnapshot) => {
      const data = dataSnapshot.val()
      if (!data) {
        return [[], { ref: collectionRef }]
      }
      const records = []
      utils.forOwn(data, (value, key) => {
        Object.defineProperty(value, mapper.idAttribute, {
          value: key
        })
        records.push(value)
      })
      const _query = new Query({
        index: {
          getAll () {
            return records
          }
        }
      })
      return [_query.filter(query).run(), { ref: collectionRef }]
    })
  }

this part is added.

Object.defineProperty(value, mapper.idAttribute, {
  value: key
})

Best,
Jovan

[jmdobry]

js-data-firebase works best when the key is part of the result set. If you use js-data-firebase to create records in firebase, then it will save the key to the result set for you. It looks like you may be loading items that were not created by js-data-firebase, and thus do not have the key in the result set.

Your suggestion of adding

Object.defineProperty(value, mapper.idAttribute, {
  value: key
})

seems like it would work, I look forward to your PR. However, couldn't it be simplified to

value[mapper.idAttribute] = key

?

[JovanT]

Hi Jason,
You are right it is simpler as you suggest,
but my idea is to make hidden property idAttribute, and now going back to it, even i think is better to attach it to prototype of the value object not to value object itself, like that we don't need to copy key data also to the the node value.
Using Object.defineProperty is creating new property which is not writable, not enumerable and can't be deleted, that is my idea.

my suggested code will look like this:

 Object.defineProperty(value.prototype, mapper.idAttribute, {
  value: key
})

if you think it is fine and you agree Ill offer even to make the change and to begin as the js-data-firebase contributor, step by step :)

Best,
Jovan.

[JovanT]

Mi Idea for creating a property on prototype is not working, so I'm going back to initial idea.
So i used Object.defineProperty just to add property attributes as i explained above. In plus we need to add check if the property already exist just not to overwrite it like:

if (!value.hasOwnProperty(mapper.idAttribute)) {
    Object.defineProperty(value, mapper.idAttribute, {
        value: key
    })
}

Best,
Jovan.