-
Notifications
You must be signed in to change notification settings - Fork 27
Description
At Remix we use a lot of geographical data, which we need to index using a spatial index, for which we use rbush. Therefore we need to insert models into rbush any time they are inserted into a collection, and removed from rbush when they are removed from the collection.
My first idea was to listen to add
and remove
events, but unfortunately they can be silenced. So the only option left is to override methods. It turns out that set
calls remove
, so overriding remove
works for that case. However, set
doesn't call add
, but add
calls set
(which is inconsistent/confusing in itself). So I end up having to do something like this:
set(...args) {
// Store what models we have before calling `set`
const previousModelIds = this.map(model => model.id);
// Call super.set()
const modelArrayOrObj = Collection.prototype.set.apply(this, args);
// See which models have been added
const models = Array.isArray(modelArrayOrObj) ? modelArrayOrObj : [modelArrayOrObj];
const addedModels = models.filter(model => !previousModelIds.includes(model.id));
// Load new models into rbush
const treeNodes = addedModels.map(createTreeNode);
this.tree.load(treeNodes);
// Return what was originally returned by `super.set()`
return modelArrayOrObj;
},
Am I missing something here? Or is this actually so inconvenient?
There are a couple of ways I see how this can be fixed. The nicest one IMO would be to have set
call add
, to be consistent with remove
. But I can see how that would be a tricky change. Another option would be to expose this.addedModels
, this.removedModels
, and this.mergedModels
, after calling set
, and have remove
call set
(for consistency). What do you think?