Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safety checks for applying properties #122

Closed
wants to merge 5 commits into from
Closed

Conversation

dphfox
Copy link
Owner

@dphfox dphfox commented Jan 30, 2022

Closes #108 and related to #34.

This adds a few safety checks to applyInstanceProps to help stop developers footgunning themselves with hard-to-debug undefined behaviour. Specifically:

  • Duplicate property key binding will throw an error. For any given key, there should be one, and only one, source of truth. Imperative-style property modification is non-idiomatic and explicitly disallowed:
local instance = New "Folder" {
    Name = "Bob"
}

Hydrate(instance) {
    Name = "Suze" --> Can't assign to 'Name' twice.
}
local instance = New "Folder" {
    [Children] = Computed(function() ... end)
}

Hydrate(instance) {
    [Children] = Computed(function() ... end) --> Can't assign to 'Children' twice.
}
  • Ambiguous parents/children will throw an error. Ambiguity is introduced when an instance with Parent defined is passed to [Children], or when one instance is passed into two [Children] definitions:
New "Folder" {
    [Children] = New "Folder" {
        Parent = workspace --> The parent of 'Folder' is ambiguous - only one Parent or [Children] is allowed at a time.
    }
}
local childA = Value(New "Part" {})
local childB = Value(nil)

New "Folder" {
    [Children] = childA
}
New "Folder" {
    [Children] = childB
}

childB:set(childA:get()) --> The parent of 'Part' is ambiguous - only one Parent or [Children] is allowed at a time.

@dphfox dphfox added the enhancement New feature or request label Feb 4, 2022
@dphfox dphfox closed this Feb 1, 2023
@dphfox dphfox deleted the pr-apply-props-safely branch February 1, 2023 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catching double child assignment
1 participant