Fusion icon indicating copy to clipboard operation
Fusion copied to clipboard

Catching double child assignment

Open dphfox opened this issue 4 years ago • 3 comments

Currently in Fusion, the following code will not error, and place both instances in folder1. This is because [Children] doesn't override a manual Parent definition:


local folder1 = New "Folder" {}

local instance1 = New "Part" {
    Parent = folder1
}

local instance2 = New "Part" {
    Parent = Value(folder1)
}

local folder2 = New "Folder" {
    [Children] = {instance1, instance2}
}

The following code also doesn't throw an error, but the parent of instance1 is undefined and prone to switching dependent on the internal implementation of Fusion:

local instance1 = New "Part" {}

local folder1 = New "Folder" {
    [Children] = {instance1}
}

local folder2 = New "Folder" {
    [Children] = {instance1}
}

I think that in any case where an instance's ancestry is defined twice, we should throw an error, including cases where we currently allow it by giving Parent priority. This is a breaking change but it's likely a really bad idea to introduce this kind of ambiguity into a codebase anyway when everything is meant to be declarative, final and guaranteed to hold at all times.

dphfox avatar Dec 24 '21 19:12 dphfox

it's likely a really bad idea to introduce this kind of ambiguity into a codebase anyway when everything is meant to be declarative, final and guaranteed to hold at all times

If you're defining an instance's ancestry twice, it can lead to some interesting issues, and I can't think of any good use cases for doing that anyways.

Additionally, if you're re-using some part of your UI you typically should be using components. If anything, erroring on the second example forces people to think about how they're setting up their UI.

For that reason I think this would be a good addition.

nottirb avatar Dec 24 '21 19:12 nottirb

Waiting for #206 to see how we should implement this.

dphfox avatar Feb 01 '23 01:02 dphfox

Unblocking this. We won't need double parent assignment - that should be impossible now - but it's still possible to attempt to parent one child into two places. I'm not sure how urgent this is though, so I'm probably not going to tackle it until down the road.

dphfox avatar Apr 15 '24 13:04 dphfox