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

[template-engine-preview-5] Transforms returning objects without _id #1617

Closed
mystor opened this issue Nov 16, 2013 · 4 comments
Closed

[template-engine-preview-5] Transforms returning objects without _id #1617

mystor opened this issue Nov 16, 2013 · 4 comments

Comments

@mystor
Copy link

mystor commented Nov 16, 2013

If you return an object without an _id property from a collection transform, it produces a very cryptic error if a cursor for the collection is later used in a template.

// javascript
Things = new Meteor.Collection('things', {
  transform: function(doc) {
    return { name: doc.name };
  }
});

Template.tpl_things.things = function() {
  return Things.find();
}
<!-- html -->
<template name="tpl_things">
  <ul>
    {{#each things}}
      <li>{{name}}</li>
    {{/each}}
  </ul>
</template>

I understand that having an _id property is useful for objects returned by a transform, but it would be nice to have a check to ensure that an _id field is returned if future code will rely on its presence.

This code runs without issue on the current release of meteor (0.6.6.3).

@avital
Copy link
Contributor

avital commented Nov 21, 2013

Thanks @mystor. Agreed that we should supply a better error in case a transform function returns documents with no _id property.

I'd just like to ask: Requiring an _id property is a change from 0.6.6.3. Given your experience with transforms, do you think there are cases when forcing the developer to supply an _id will be difficult? Or would it always just be passing in the _id from the original document being transformed?

@avital
Copy link
Contributor

avital commented Nov 21, 2013

I chatted with @sixolet and @glasser about this and we're probably going to make the following changes to transform:

  1. Only allow objects to be returned
  2. After the transform function is called, an _id property is added to the returned object. The _id is taken from the original untransformed document. (If transform already returns an object with an _id field, verify that it is the same as the original document's _id)

This will resolve the problem without requiring users to specify _id manually.

@mystor
Copy link
Author

mystor commented Nov 21, 2013

I think that this is a good option. If the _id property is required, then it makes sense to force the transform function to return objects, and to manually set the _id property to prevent user error.

The only situation which I can think of for wanting to have something without an _id property would be a document like the following:

{
  "_id": "asdfgh",
  "text": "abc123"
}

Where the transform would just return the string "abc123", to remove the requirement for a property access (which could fail if Collection.findOne() returned undefined). I feel that preventing that use case is probably good, as it encourages better practices and ensures that people don't build their projects in an unmaintainable way. A transform without an _id property means that removing and updating the object will be much more difficult.

@avital
Copy link
Contributor

avital commented Dec 3, 2013

Fixed on fe661ee

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

No branches or pull requests

2 participants