objection.js: $relatedQuery shouldn't mutate loaded relations

I find that $relatedQuery.findById and $relatedQuery.insert will mutate the relations loaded on a model instance. I find that this isn’t particularly useful & tends to create more problems than it avoids.

Consider e.g

Loading a model with a filtered relation. Any insert or find operation on the same relation using $relatedQuery breaks the filter.
A ‘shortcut’ relation that is defined only for syntactic sugar (e.g often this is the inverse of a more common access pattern - e.g find genre related to this artist, where you usually access genre->artist & don’t really want genre nested under artist when serialized etc.) - querying with $relatedQuery you now have an unwanted piece of data non-obviously nested in your model

My proposal would be to disable this behaviour as I don’t think a complete solution exists here.

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 21 (7 by maintainers)

Most upvoted comments

@drew-r @jordansexton We have two people on this thread and already two different opinions. I think everyone will have their own about this one.

I think it’s even more inconsistent if $relatedQuery('foo').insert adds the result, but no other operation does. Now the reasoning is add the values whenever we can. I think the only consistent change is to remove the mutation altogether.

@jordansexton What if we add helpers like this:

const insertedChild = await person.$relatedQuery('children').insert(child)
person.$setRelated('children', insertedChild);

const insertedChildren = await person.$relatedQuery('children').insert([child1, child2, child3])
person.$appendRelated('children', insertedChild);

Those helpers would do the right thing based on relation type and whether the relations are empty or not.

koskimas on Dec 8, 2017

@jordansexton @drew-r What if I add two booleans:

Model.relatedFindQueryMutates = true;
Model.relatedInsertQueryMutates = true;

and leave them to true by default now?

koskimas on Dec 20, 2017

@VladShcherbin Yes I’m aware of that, but I’d really like to move to 1.0 as soon as possible.

koskimas on Nov 11, 2017