jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.09k stars 5.39k forks source link

TypeError: existing.set is not a function #4224

Closed PeterW570 closed 2 years ago

PeterW570 commented 4 years ago
const collection = new Backbone.Collection();
const model = new Backbone.Model({
    id: 'hasOwnProperty'
});
collection.set([model]); // TypeError: existing.set is not a function

In Collection#set, it does: var existing = this.get(model); which in this case is setting existing to Object#hasOwnProperty rather than undefined because Collection#get is returning the result ofthis._byId['hasOwnProperty']

Have reproduced this bug in a codepen here.

paulfalgout commented 4 years ago

This seems like an odd thing to fix. This to me seems unlikely in general, but fixing it would add at least some cost to most actions across backbone where this issue is never likely to arise. If anything the best fix to me seems to be something like adding to the docs "ids cannot be members of Array.prototype.String"

PeterW570 commented 4 years ago

Can also cause the same bug by setting it to a different attribute and then setting idAttribute of the model to that, e.g:

  const keyedModel = Backbone.Model.extend({ idAttribute: 'key' });
  const keyedModelCollection = Backbone.Collection.extend({ model: keyedModel });
  const collection = new keyedModelCollection();
  const model = new keyedModel({ key: 'hasOwnProperty' });
  collection.set([model]); // TypeError: existing.set is not a function

Agree it's an edge case, but I think it's a valid use case if the string is a valid unique identifier in the database

jgonggrijp commented 2 years ago

As far as I'm concerned, this isn't specific to Backbone; it's on the list of things you should do nowhere in JavaScript. Of course, it's fine if you learn by experimentation that doing stuff like this breaks things, but this doesn't need documentation in Backbone, let alone a fix.

As for unique identifiers in databases, I expect those to be serial numbers or something more advanced like a UUID. Certainly not names of JavaScript's global object methods.