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

Why are models without IDs added to the store? #411

Closed
bent opened this Issue Oct 24, 2013 · 2 comments

Comments

Projects
None yet
3 participants
@bent

bent commented Oct 24, 2013

Is there was any particular reason that Backbone-Relational models without IDs are added to the store? The reason I ask is that I'm thinking of submitting a pull request where they only get added if/when they get an ID, but thought I'd check if there was any obvious reason that it wouldn't work.

Our use-case is that, in addition to using Backbone.Relational for managing regular related data, I'm also using it for deserializing and serializing 'view-models' that don't actually have IDs. At the moment these related view models go into the store, even though they're only used on a one-off basis. I don't need to ensure object uniqueness for them, and I don't want them to hang around in memory.

On another project where we weren't using Backbone-Relational, we wrote an identity map where a model only got added once it got an ID (see https://github.com/shinetech/backbone-identity-map/blob/master/backbone-identity-map.js). I'd be happy to write a pull request to bring this sort of behaviour into Backbone-Relational.

Thoughts?

Ben

@philfreo

This comment has been minimized.

Show comment
Hide comment
@philfreo

philfreo Oct 25, 2013

Collaborator

Models that will never have an ID definitely don't need to be in the Store.

Just probably need to think about the cases where a new model gets an ID later.

Collaborator

philfreo commented Oct 25, 2013

Models that will never have an ID definitely don't need to be in the Store.

Just probably need to think about the cases where a new model gets an ID later.

@PaulUithol

This comment has been minimized.

Show comment
Hide comment
@PaulUithol

PaulUithol Oct 27, 2013

Owner

I don't think there's a particular reason they need to be in the store. However, up until now, we've just been using the simplest possible solution of putting every model in on creation.

We already do checking for id changes in set; it shouldn't be too hard to modify that to the behavior you describe. You're welcome to try; sounds like it could be a performance win in some use cases.

Owner

PaulUithol commented Oct 27, 2013

I don't think there's a particular reason they need to be in the store. However, up until now, we've just been using the simplest possible solution of putting every model in on creation.

We already do checking for id changes in set; it shouldn't be too hard to modify that to the behavior you describe. You're welcome to try; sounds like it could be a performance win in some use cases.

nl0 added a commit to nl0/Backbone-relational that referenced this issue Feb 25, 2014

Merge upstream
* upstream/master: (35 commits)
  Upgrade qunit
  Fix subModels not being populated properly on Underscore 1.6.0, due to `_.each` api change.
  Fire change events right away if we're not in a nested scenario. Fixes #427
  Document `store.unregister`
  `store.unregister` now also accepts collections or a model type
  Change `findOrCreate` to pass `parsedAttributes` to `build`, and adjust tests to reflect change.
  Remove unnecessary locking
  Update change log
  Add test for models not being added to store until they get an id
  Change where we listen to `relational:unregister`.
  Only add models with an id to the store. Closes #411
  Fix a bug in the (crude) performance test
  Add a small test for `clear`
  Fix collection return values when setting/removing `[]` and `null`. Ref #419
  Clarify the behavior of `findOrCreate` when it just a receives a scalar value. Ref #399
  Proper return values on collection methods for Backbone 1.1. Closes #419
  Submodels: accommodate multiple 'type' keys for the same submodel. Closes #429
  Hmm, fix spaces/tabs mix in the example
  Update a few version numbers in the docs to 0.8.7
  Backbone-relational 0.8.7
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment