diff --git a/CHANGELOG.md b/CHANGELOG.md index 83b4399..61b8fd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `create("ClassName", { props })` and `create(Instance, { props })` syntax. - `cleanup()` now accepts `thread` types. +- Implicit effects to set children can now recursively create more implicit + effects to set children. ### Changed - A scope can no longer be destroyed while it is active. Strict mode will check for this. +- Implicit effects to set children now unparent all children when the effect is + destroyed. ### Removed diff --git a/docs/api/strict-mode.md b/docs/api/strict-mode.md index 630a7bd..6f9ee80 100644 --- a/docs/api/strict-mode.md +++ b/docs/api/strict-mode.md @@ -20,7 +20,7 @@ Currently, strict mode will: 4. Checks for `values()` input having duplicate values. 5. Checks for duplicate nested properties at same depth. 6. Checks for destruction of an active scope. -7. Better error reporting and stack traces + creation traces of property bindings. +7. Better error reporting and stack traces. By rerunning reactive scopes twice each time they update, it helps ensure that computations are pure, and that any cleanup is done correctly. diff --git a/src/apply.luau b/src/apply.luau index 754c0d2..2b30ba6 100644 --- a/src/apply.luau +++ b/src/apply.luau @@ -1,10 +1,8 @@ local typeof = game and typeof or require "../test/mock".typeof :: never -local Vector2 = game and Vector2 or require "../test/mock".Vector2 :: never -local UDim2 = game and UDim2 or require "../test/mock".UDim2 :: never local flags = require "./flags" local throw = require "./throw" -local bind = require "./bind" +local implicit_effect = require "./implicit_effect" local _, is_action = require "./action"() local graph = require "./graph" type Node = graph.Node @@ -79,14 +77,14 @@ local function process_properties(properties: Map, instance: I table.insert(cache.events, property) -- add event name to buffer table.insert(cache.events, value :: () -> ()) -- add event listener to buffer else - bind.property(instance, property, value :: () -> ()) -- create implicit effect for property + implicit_effect.property(instance, property, value :: () -> ()) -- create implicit effect for property end else (instance :: any)[property] = value -- set property end elseif type(property) == "number" then if type(value) == "function" then - bind.children(instance, value :: () -> ArrayOrV) -- bind children + implicit_effect.children(instance, value :: () -> ArrayOrV) -- bind children elseif type(value) == "table" then if is_action(value) then table.insert(cache.actions[(value :: any).priority], (value :: any).callback :: () -> ()) -- add action to buffer @@ -138,16 +136,14 @@ local function apply(instance: T & Instance, properties: { [unknown]: unknown end end - -- finally set parent if any if parent then if type(parent) == "function" then - bind.parent(instance, parent :: () -> Instance) + implicit_effect.parent(instance, parent :: () -> Instance) else instance.Parent = parent :: Instance end end - -- clear caches table.clear(events) for _, queued in next, actions do table.clear(queued) end if flags.strict then table.clear(nested_debug) end diff --git a/src/bind.luau b/src/bind.luau deleted file mode 100644 index ff32bcb..0000000 --- a/src/bind.luau +++ /dev/null @@ -1,103 +0,0 @@ -local graph = require "./graph" -type Node = graph.Node -local create_node = graph.create_node -local assert_stable_scope = graph.assert_stable_scope -local evaluate_node = graph.evaluate_node - -function create_implicit_effect(updater: (T) -> T, binding: T) - evaluate_node(create_node(assert_stable_scope(), updater, binding)) -end - -type PropertyBinding = { - instance: Instance, - property: string, - source: () -> unknown -} - -local function update_property_effect(p: PropertyBinding) - (p.instance :: any)[p.property] = p.source() - return p -end - -type ParentBinding = { - instance: Instance, - parent: () -> Instance -} - -local function update_parent_effect(p: ParentBinding) - p.instance.Parent = p.parent() - return p -end - -type ChildrenBinding = { - instance: Instance, - cur_children_set: { [Instance]: true }, - new_children_set: { [Instance]: true }, - children: () -> Instance | { Instance } -} - -type ArrayOrV = V | { V } -local function update_children_effect(p: ChildrenBinding) - local cur_children_set: { [Instance]: true } = p.cur_children_set -- cache of all children parented before update - local new_child_set: { [Instance]: true } = p.new_children_set -- cache of all children parented after update - - local new_children = p.children() -- all (and only) children that should be parented after this update - - if type(new_children) ~= "table" then - new_children = { new_children } - end - - local function process_child(child: ArrayOrV) - if type(child) == "table" then - for _, child in next, child do - process_child(child) - end - else - if new_child_set[child] then return end -- stops redundant reparenting - - new_child_set[child] = true -- record child set from this update - if not cur_children_set[child] then - child.Parent = p.instance -- if child wasn't already parented then parent it - else - cur_children_set[child] = nil -- remove child from cache if it was already in cache - end - end - end - - process_child(new_children) - - for child in next, cur_children_set do - child.Parent = nil -- unparent all children that weren't in the new children set - end - - table.clear(cur_children_set) -- clear cache, preserve capacity - p.cur_children_set, p.new_children_set = new_child_set, cur_children_set - - return p -end - -return { - property = function(instance, property, source) - return create_implicit_effect(update_property_effect, { - instance = instance, - property = property, - source = source - }) - end, - - parent = function(instance, parent) - return create_implicit_effect(update_parent_effect, { - instance = instance, - parent = parent - }) - end, - - children = function(instance, children) - return create_implicit_effect(update_children_effect, { - instance = instance, - cur_children_set = {}, - new_children_set = {}, - children = children - }) - end -} diff --git a/src/implicit_effect.luau b/src/implicit_effect.luau new file mode 100644 index 0000000..5332662 --- /dev/null +++ b/src/implicit_effect.luau @@ -0,0 +1,119 @@ +local graph = require "./graph" +type Node = graph.Node +local create_node = graph.create_node +local assert_stable_scope = graph.assert_stable_scope +local get_scope = graph.get_scope +local evaluate_node = graph.evaluate_node +local push_cleanup = graph.push_cleanup + +local function update_property_effect(p: { + instance: Instance, + property: string, + source: () -> unknown +}) + (p.instance :: any)[p.property] = p.source() + return p +end + +local function update_parent_effect(p: { + instance: Instance, + source: () -> Instance +}) + p.instance.Parent = p.source() + return p +end + +local function update_children_effect(p: { + instance: Instance, + cur_children_set: { [Instance]: true }, + new_children_set: { [Instance]: true }, + source: () -> Instance | { Instance } +}) + local cur_children_set: { [Instance]: true } = p.cur_children_set -- cache of all children parented before update + local new_children_set: { [Instance]: true } = p.new_children_set -- cache of all children parented after update + + local new_children = p.source() -- all (and only) children that should be parented after this update + + local function process_child(child: Instance | { Instance }) + if type(child) == "userdata" then + if new_children_set[child] then return end -- stops redundant reparenting + + new_children_set[child] = true -- record child set from this update + if not cur_children_set[child] then + child.Parent = p.instance -- if child wasn't already parented then parent it + else + cur_children_set[child] = nil -- remove child from cache if it was already in cache + end + elseif type(child) == "table" then + for _, child in next, child do + process_child(child) + end + elseif type(child) == "function" then + local node = create_node(assert(get_scope()), update_children_effect, { + instance = p.instance, + cur_children_set = {}, + new_children_set = {}, + source = child + }) + + evaluate_node(node) + + push_cleanup(assert(get_scope()), function() + for child in node.cache.cur_children_set do + child.Parent = nil + end + end) + end + end + + process_child(new_children) + + for child in next, cur_children_set do + child.Parent = nil -- unparent all children that weren't in the new children set + end + + table.clear(cur_children_set) -- clear cache, preserve capacity + p.cur_children_set, p.new_children_set = new_children_set, cur_children_set + + return p +end + +return { + property = function(instance, property, source) + local node = create_node(assert_stable_scope(), update_property_effect, { + instance = instance, + property = property, + source = source + }) + evaluate_node(node) + return node + end, + + parent = function(instance, parent) + local node = create_node(assert_stable_scope(), update_parent_effect, { + instance = instance, + source = parent + }) + evaluate_node(node) + return node + end, + + children = function(instance, children) + local node = create_node(assert_stable_scope(), update_children_effect, { + instance = instance, + cur_children_set = {}, + new_children_set = {}, + source = children + }) + + evaluate_node(node) + + push_cleanup(assert_stable_scope(), function() + for child in node.cache.cur_children_set do + child.Parent = nil + end + end) + + return node + end +} diff --git a/test/tests.luau b/test/tests.luau index b11e47a..d4c95ae 100644 --- a/test/tests.luau +++ b/test/tests.luau @@ -996,6 +996,48 @@ TEST("create()", wrap_root(function() CHECK(not obj:FindFirstChild("e")) end + do CASE "nested children source effect" + local a = create "Frame" { Name = "a" } :: Instance + local b = create "Frame" { Name = "b" } :: Instance + local c = create "Frame" { Name = "c" } :: Instance + + local nested_children = source { b, c } + local children = source { a :: Instance | () -> { Instance }, nested_children } + + local parent = create "Frame" { + Name = "parent", + children + } + + CHECK(parent:FindFirstChild "a") + CHECK(parent:FindFirstChild "b") + CHECK(parent:FindFirstChild "c") + nested_children {} + CHECK(parent:FindFirstChild "a") + CHECK(not parent:FindFirstChild "b") + CHECK(not parent:FindFirstChild "c") + nested_children { b } + CHECK(parent:FindFirstChild "a") + CHECK(parent:FindFirstChild "b") + CHECK(not parent:FindFirstChild "c") + children { a } + CHECK(parent:FindFirstChild "a") + CHECK(not parent:FindFirstChild "b") + CHECK(not parent:FindFirstChild "c") + nested_children { b, c } + CHECK(parent:FindFirstChild "a") + CHECK(not parent:FindFirstChild "b") + CHECK(not parent:FindFirstChild "c") + children { a :: Instance | () -> { Instance }, nested_children } + CHECK(parent:FindFirstChild "a") + CHECK(parent:FindFirstChild "b") + CHECK(parent:FindFirstChild "c") + nested_children { c } + CHECK(parent:FindFirstChild "a") + CHECK(not parent:FindFirstChild "b") + CHECK(parent:FindFirstChild "c") + end + do CASE "garbage collection test" local wref