meteor-utilities / react-list-container

Smart containers for React & Meteor
32 stars 8 forks source link

Why is selector required for Documents? #5

Closed genyded closed 8 years ago

genyded commented 8 years ago

Maybe I am missing something, but if I have something like:

Meteor.publish('currentItem', function (id) {
  return Items.find({_id: id});
});

...why would I need a selector on the subscription? For Documents, it's a required property and although I can pass in an empty object - that seems like unnecessary code. Is there some logic I am missing behind forcing that property for Documents?

SachaG commented 8 years ago

Well you need the selector to find the document on the client, so I assume you mean the terms argument? I suppose we could reuse the selector, but it feels more consistent with the ListContainer to just let people specify whatever they want.

Also who knows, maybe people will want to do weird things like getting back one random document?

genyded commented 8 years ago

You are correct and I already have another issue open for terms not being required. If I get some time I'll submit a PR for that one, but it's a really quick fix to remove isRequired for that PropType in both source files.

SachaG commented 8 years ago

Yeah I'll fix it :)