feat: set $parent on create and set

[philippfromme]

This pull request implements a simple approach to maintaining the $parent property.

What it does ✅

Sets $parent when using Moddle#create

var child = model.create('props:Parent', {
  child: model.create('props:Child')
});

console.log(parent.get('child').$parent); ✅ 

Sets $parent when using Base#set

var parent = model.create('props:Parent'),
    child = model.create('props:Child');

parent.set('child', child);

console.log(child.$parent); ✅ 

Unsets $parent when using Base#set

const child = model.create('props:Child');

const parent = model.create('props:Parent', {
  child
});

parent.set('child', undefined);

console.log(child.$parent); ✅ 

What it doesn't do ⛔

Setting parent without using API

var parent = model.create('props:Parent'),
    child = model.create('props:Child'),
    otherChild = model.create('props:Child');

parent.child = child;
console.log(child.$parent); ⛔ 

parent.children.push(otherChild);
console.log(otherChild.$parent); ⛔ 

Unsetting parent without using API

var parent = model.create('props:Parent'),
    child = model.create('props:Child');

parent.set('child', child);

parent.child = undefined;

console.log(child.$parent); ⛔ 

This is the approach I'd prefer. Every model element has a clearly defined API. Using this API is the preferred way of working with model elements. Accessing properties without using the API (parent.child = child;) is possible but not necessarily intended. I consider setting and unsettling the parent using proxies as magic and, therefore, wouldn't implement that.

---

Closes #41

[philippfromme]

@barmac @marstamm @nikku @smbea, feel free to have a look and share your sentiment regarding my proposal.

[nikku]

Great initiative!

I consider setting and unsettling the parent using proxies as magic and, therefore, wouldn't implement that.

Not sure from this comment: Did you consider proxies? I'd love us to consider them, and discard if we found them unsuitable.

Unsetting parent

I think we cannot ship without that. If $parent is managed by the infrastructure then I'd assume that unsetting (via a well known path) does unset $parent, too. We'll otherwise not have a way to find out if an element is attached to the tree.

[philippfromme]

Not sure from this comment: Did you consider proxies? I'd love us to consider them, and discard if we found them unsuitable.

I wanted to start the conversation first. I was anticipating the question about proxies. I'm not to fond of them. I agree that it's not great to set the $parent but not have any way of unsetting it. Maybe a Base#unset method is missing.

[nikku]

Maybe a Base#unset method is missing.

Usually we set to undefined instead. But Base#unset would be valid, too. We'd just need to make sure there exists a way to properly unset, too.

[philippfromme]

Usually we set to undefined instead.

Now that I'm thinking about it, this could actually work. When calling set with undefined we unset $parent of all existing properties before removing them. I'll try that out.

[philippfromme]

I'll try that out.

Done.

[nikku]

@philippfromme Did you try this against diagram-js / bpmn-moddle / bpmn-js? Great code-bases to integration-test against.

[philippfromme]

@philippfromme Did you try this against diagram-js / bpmn-moddle / bpmn-js? Great code-bases to integration-test against.

No I haven't done that yet. I can do that, though. I assume that introducing this feature would allow us to remove a lot of child.$parent = parent across our libraries.

[barmac]

Let's verify that it does not break anything in the downstream libraries.

[philippfromme]

Let's verify that it does not break anything in the downstream libraries.

That's what I'm doing.

[philippfromme]

I will pick this up again once I have time.

[marstamm]

I will pick this up again once I have time.

I converted the PR to draft for now so we don't get notification on Slack about it