From c4a0180a2f60c90c3be1a433f2dd393099a0b0ce Mon Sep 17 00:00:00 2001 From: aaron <83140718+centau@users.noreply.github.com> Date: Mon, 18 Sep 2023 09:40:54 +0100 Subject: [PATCH] Improve error messages for derived sources There seemed to be confusion over reactive-scoping (likely due to incomplete docs), so I've added more explicit error messages when deriving in non-reactive scopes to warn the user to use `root()` or `mount()`. --- src/bind.luau | 8 ++------ src/cleanup.luau | 5 +++-- src/derive.luau | 8 ++------ src/effect.luau | 8 ++------ src/graph.luau | 15 ++++++++++++++- src/maps.luau | 12 +++--------- src/spring.luau | 7 ++----- src/switch.luau | 9 +++------ test/tests.luau | 1 - 9 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/bind.luau b/src/bind.luau index bcd0701..c614590 100644 --- a/src/bind.luau +++ b/src/bind.luau @@ -1,12 +1,11 @@ if not game then script = require "test/relative-string" end -local throw = require(script.Parent.throw) local trace = require(script.Parent.trace) local flags = require(script.Parent.flags) local graph = require(script.Parent.graph) type Node = graph.Node local create_node = graph.create_node -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local evaluate_node = graph.evaluate_node local set_owner = graph.set_owner @@ -33,10 +32,7 @@ function create_binding(updater: (T) -> T, binding: T) end - local owner = get_scope() - if not owner then - throw("cannot bind property in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() local node = create_node(binding, updater) diff --git a/src/cleanup.luau b/src/cleanup.luau index 05a516f..0bfb6c1 100644 --- a/src/cleanup.luau +++ b/src/cleanup.luau @@ -7,8 +7,9 @@ local add_cleanup = graph.add_cleanup local function cleanup(callback: () -> ()) local scope = get_scope() - if not scope then - throw("cannot cleanup in a non-reactive scope") + + if not scope then + throw "cannot cleanup in a non-reactive scope" end; assert(scope) add_cleanup(scope, callback) diff --git a/src/derive.luau b/src/derive.luau index 29b915a..49094d5 100644 --- a/src/derive.luau +++ b/src/derive.luau @@ -1,18 +1,14 @@ if not game then script = require "test/relative-string" end -local throw = require(script.Parent.throw) local graph = require(script.Parent.graph) local create_node = graph.create_node local set_owner = graph.set_owner local track = graph.track -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local evaluate_node = graph.evaluate_node local function derive(source: () -> T): () -> T - local owner = get_scope() - if not owner then - throw("cannot derive in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() local node = create_node(false :: any, source) diff --git a/src/effect.luau b/src/effect.luau index bc53660..43b12ab 100644 --- a/src/effect.luau +++ b/src/effect.luau @@ -1,17 +1,13 @@ if not game then script = require "test/relative-string" end -local throw = require(script.Parent.throw) local graph = require(script.Parent.graph) local create_node = graph.create_node -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local evaluate_node = graph.evaluate_node local set_owner = graph.set_owner local function effect(callback: (T) -> T, initial_value: T) - local owner = get_scope() - if not owner then - throw("cannot create effect in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() local node = create_node(initial_value, callback) diff --git a/src/graph.luau b/src/graph.luau index 20140bd..77db497 100644 --- a/src/graph.luau +++ b/src/graph.luau @@ -45,6 +45,17 @@ local function get_scope(): Node? return scopes[scopes.n] end +local function get_owning_scope(): Node + local scope = get_scope() + if not scope then + local caller_name = debug.info(2, "n") + return throw(`cannot use {caller_name}() in non-reactive scope, must be used within a root() or mount() callback`) + elseif scope.effect then + throw("owning scope is not stable; are you trying to derive a new source from within a side-effect?") + end + return scope +end + local function add_child(parent: StartNode, child: Node) table.insert(parent, child) table.insert(child.parents, parent) @@ -149,6 +160,7 @@ local function evaluate_node(node: Node) return cur_value ~= new_value -- node has changed value end +-- todo: case where owner is set from an untrack call within an effectful node, children clearing local function update_from(node: StartNode, n0: number) if not node[1] then return end @@ -169,7 +181,7 @@ local function update_from(node: StartNode, n0: number) -- evaluate all queued children for i = n0 + 1, n do - local child = update_queue[i] + local child = update_queue[i] -- todo: error: index boolean if not child.effect then continue end if evaluate_node(child) then @@ -213,6 +225,7 @@ return table.freeze { close_scope = close_scope, evaluate_node = evaluate_node, get_scope = get_scope, + get_owning_scope = get_owning_scope, add_cleanup = add_cleanup, set_owner = set_owner, destroy = destroy, diff --git a/src/maps.luau b/src/maps.luau index da5c5d7..1649039 100644 --- a/src/maps.luau +++ b/src/maps.luau @@ -10,7 +10,7 @@ local create_start_node = graph.create_start_node local set_owner = graph.set_owner local track = graph.track local update = graph.update -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local open_scope = graph.open_scope local close_scope = graph.close_scope local evaluate_node = graph.evaluate_node @@ -28,10 +28,7 @@ local function check_primitives(t: {}) end local function indexes(input: () -> Map, transform: (() -> VI, K) -> VO): () -> { VO } - local owner = get_scope() - if not owner then - throw("cannot derive in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() local subowner = create_node(false, false) set_owner(subowner, owner) @@ -125,10 +122,7 @@ local function indexes(input: () -> Map, transform: (() -> VI, end local function values(input: () -> Map, transform: (VI, () -> K) -> VO): () -> { VO } - local owner = get_scope() - if not owner then - throw("cannot derive in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() local subowner = create_node(false, false) set_owner(subowner, owner) diff --git a/src/spring.luau b/src/spring.luau index 9503e15..e60e3ae 100644 --- a/src/spring.luau +++ b/src/spring.luau @@ -27,7 +27,7 @@ type Node = graph.Node type StartNode = graph.StartNode local create_node = graph.create_node local create_start_node = graph.create_start_node -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local evaluate_node = graph.evaluate_node local update = graph.update local set_owner = graph.set_owner @@ -150,10 +150,7 @@ local springs: { [SpringData]: StartNode } = {} setmetatable(springs, { __mode = "v" }) local function spring(source: () -> T, period: number?, damping_ratio: number?): () -> T - local owner = get_scope() - if not owner then - throw("cannot derive in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() -- https://en.wikipedia.org/wiki/Damping diff --git a/src/switch.luau b/src/switch.luau index 31c2b4a..421d583 100644 --- a/src/switch.luau +++ b/src/switch.luau @@ -9,19 +9,16 @@ local evaluate_node = graph.evaluate_node local set_owner = graph.set_owner local track = graph.track local destroy = graph.destroy -local get_scope = graph.get_scope +local get_owning_scope = graph.get_owning_scope local open_scope = graph.open_scope local close_scope = graph.close_scope type Map = { [K]: V } local function switch(source: () -> T): (map: Map U)?)>) -> () -> U? - return function(map) - local owner = get_scope() - if not owner then - throw("cannot switch in non-reactive scope") - end; assert(owner) + local owner = get_owning_scope() + return function(map) local last_scope: Node? local last_component: (() -> U)? diff --git a/test/tests.luau b/test/tests.luau index 4d8a52c..c8844f8 100644 --- a/test/tests.luau +++ b/test/tests.luau @@ -464,7 +464,6 @@ TEST("derive()", wrap_root(function() local a = source(0) local destroy = vide.mount(function() - local _b = derive(function() cleanup(function() count += 1