From 23e1509e846a5bd8a913d51dce192dbd64e014d1 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sat, 4 May 2019 15:48:15 -0400 Subject: [PATCH 01/13] joinBindings implementation --- src/Binding.lua | 42 ++++++++++++++++++++++++++++++++++++++++++ src/Binding.spec.lua | 41 +++++++++++++++++++++++++++++++++++++++++ src/init.lua | 1 + src/init.spec.lua | 1 + 4 files changed, 85 insertions(+) diff --git a/src/Binding.lua b/src/Binding.lua index 06a9e267..3a6c94a7 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -11,6 +11,19 @@ local function identity(value) return value end +--[[ + Maps a table of bindings to their respective values. Used in Binding.join. +]] +local function mapBindingsToValues(bindings) + local values = {} + + for key, binding in pairs(bindings) do + values[key] = binding:getValue() + end + + return values +end + local Binding = {} --[[ @@ -146,4 +159,33 @@ function Binding.create(initialValue) return binding, setter end +--[[ + Creates a new binding which updates when any of the upstream bindings + updates, which can be further mapped into any value. This function will + be exposed to users of Roact. +]] +function Binding.join(bindings) + local joinedBinding, setter = Binding.create(mapBindingsToValues(bindings)) + local internalData = joinedBinding[InternalData] + + local upstreamConnections = {} + internalData.upstreamConnections = upstreamConnections + + internalData.upstreamDisconnect = function() + for _, disconnect in pairs(upstreamConnections) do + disconnect() + end + end + + local function updateBinding() + setter(mapBindingsToValues(bindings)) + end + + for key, binding in pairs(bindings) do + upstreamConnections[key] = Binding.subscribe(binding, updateBinding) + end + + return joinedBinding +end + return Binding \ No newline at end of file diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index d02fff84..7cf73408 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -21,6 +21,47 @@ return function() end) end) + describe("Binding.join", function() + it("should properly output values", function() + local binding1, update1 = Binding.create(1) + local binding2, update2 = Binding.create(2) + + local joinedBinding = Binding.join({ + binding1, + binding2, + }) + + local bindingValue = joinedBinding:getValue() + expect(bindingValue).to.be.a("table") + expect(bindingValue[1]).to.equal(1) + expect(bindingValue[2]).to.equal(2) + end) + + it("should update when any one of the subscribed bindings updates", function() + local binding1, update1 = Binding.create(1) + local binding2, update2 = Binding.create(2) + + local joinedBinding = Binding.join({ + binding1, + binding2, + }) + + local spy = createSpy() + local disconnect = Binding.subscribe(joinedBinding, spy.value) + + expect(spy.callCount).to.equal(0) + + update1(3) + + expect(spy.callCount).to.equal(1) + + local bindingValue = spy.values[1] + expect(bindingValue).to.be.a("table") + expect(bindingValue[1]).to.equal(3) + expect(bindingValue[2]).to.equal(2) + end) + end) + describe("Binding object", function() it("should provide a getter and setter", function() local binding, update = Binding.create(1) diff --git a/src/init.lua b/src/init.lua index cbaa6f0a..f002f975 100644 --- a/src/init.lua +++ b/src/init.lua @@ -22,6 +22,7 @@ local Roact = strict { Portal = require(script.Portal), createRef = require(script.createRef), createBinding = Binding.create, + joinBindings = Binding.join, Change = require(script.PropMarkers.Change), Children = require(script.PropMarkers.Children), diff --git a/src/init.spec.lua b/src/init.spec.lua index 7b23a173..7fcf79c8 100644 --- a/src/init.spec.lua +++ b/src/init.spec.lua @@ -7,6 +7,7 @@ return function() createFragment = "function", createRef = "function", createBinding = "function", + joinBindings = "function", mount = "function", unmount = "function", update = "function", From a7747702388d3411f398179ffa44f96d596c0b90 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sat, 4 May 2019 16:00:17 -0400 Subject: [PATCH 02/13] Remove whitespace from empty line --- src/Binding.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Binding.lua b/src/Binding.lua index 3a6c94a7..96d92f5d 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -170,7 +170,7 @@ function Binding.join(bindings) local upstreamConnections = {} internalData.upstreamConnections = upstreamConnections - + internalData.upstreamDisconnect = function() for _, disconnect in pairs(upstreamConnections) do disconnect() From 9100db151f8bc6bf54beb8701500b8083a903720 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sat, 4 May 2019 16:03:06 -0400 Subject: [PATCH 03/13] Fix warnings in Binding tests --- src/Binding.spec.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index 7cf73408..dbb3d7b5 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -23,8 +23,8 @@ return function() describe("Binding.join", function() it("should properly output values", function() - local binding1, update1 = Binding.create(1) - local binding2, update2 = Binding.create(2) + local binding1 = Binding.create(1) + local binding2 = Binding.create(2) local joinedBinding = Binding.join({ binding1, @@ -47,18 +47,18 @@ return function() }) local spy = createSpy() - local disconnect = Binding.subscribe(joinedBinding, spy.value) + Binding.subscribe(joinedBinding, spy.value) expect(spy.callCount).to.equal(0) - update1(3) - expect(spy.callCount).to.equal(1) + update2(4) + expect(spy.callCount).to.equal(2) local bindingValue = spy.values[1] expect(bindingValue).to.be.a("table") expect(bindingValue[1]).to.equal(3) - expect(bindingValue[2]).to.equal(2) + expect(bindingValue[2]).to.equal(4) end) end) From 10c00ef4437f20558e1c93c65f1b7ef8e51bceec Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 02:21:05 -0400 Subject: [PATCH 04/13] Use getValue instead of spy.value in Binding tests --- src/Binding.spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index dbb3d7b5..be03f329 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -55,7 +55,7 @@ return function() update2(4) expect(spy.callCount).to.equal(2) - local bindingValue = spy.values[1] + local bindingValue = joinedBinding:getValue() expect(bindingValue).to.be.a("table") expect(bindingValue[1]).to.equal(3) expect(bindingValue[2]).to.equal(4) From 681ac482853d00536e01f2dc1ddc568d8a9e254a Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 02:21:14 -0400 Subject: [PATCH 05/13] Refactor binding internals --- src/Binding.lua | 89 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/src/Binding.lua b/src/Binding.lua index 96d92f5d..6bc59ac9 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -45,13 +45,13 @@ function bindingPrototype:getValue() --[[ If our source is another binding but we're not subscribed, we'll - return the mapped value from our upstream binding. + return the mapped value from our upstream binding(s). This allows us to avoid subscribing to our source until someone has subscribed to us, and avoid creating dangling connections. ]] - if internalData.upstreamBinding ~= nil and internalData.upstreamDisconnect == nil then - return internalData.valueTransform(internalData.upstreamBinding:getValue()) + if internalData.upstreamBindingCount > 0 then + return internalData.valueTransform(self:__getValueFromUpstreamBindings()) end return internalData.value @@ -66,13 +66,49 @@ function bindingPrototype:map(valueTransform) end local binding = Binding.create(valueTransform(self:getValue())) + local internalData = binding[InternalData] - binding[InternalData].valueTransform = valueTransform - binding[InternalData].upstreamBinding = self + internalData.valueTransform = valueTransform + + internalData.upstreamBindings.source = self + internalData.upstreamBindingCount = internalData.upstreamBindingCount + 1 return binding end +--[[ + Determines the final (not yet transformed) value from upstream bindings +]] +function bindingPrototype:__getValueFromUpstreamBindings() + local internalData = self[InternalData] + local newValue = mapBindingsToValues(internalData.upstreamBindings) + + if not internalData.isJoinedBinding then + --[[ + If this is not a joined binding, there will always only be one upstream + binding. + + To ensure that joined bindings with a single upstream binding always + result in a table, we use the internal variable isJoinedBinding + ]] + local _, value = next(newValue) + newValue = value + end + + return newValue +end + +--[[ + Disconnects all connections to upstream bindings +]] +function bindingPrototype:__upstreamDisconnect() + local internalData = self[InternalData] + + for _, disconnect in pairs(internalData.upstreamConnections) do + disconnect() + end +end + --[[ Update a binding's value. This is only accessible by Roact. ]] @@ -93,13 +129,17 @@ function Binding.subscribe(binding, handler) --[[ If this binding is mapped to another and does not have any subscribers, - we need to create a subscription to our source binding so that updates + we need to create subscriptions to our source bindings so that updates get passed along to us ]] - if internalData.upstreamBinding ~= nil and internalData.subscriberCount == 0 then - internalData.upstreamDisconnect = Binding.subscribe(internalData.upstreamBinding, function(value) - Binding.update(binding, value) - end) + if internalData.upstreamBindingCount > 0 and internalData.subscriberCount == 0 then + local function upstreamCallback() + Binding.update(binding, binding:__getValueFromUpstreamBindings()) + end + + for _, binding in pairs(internalData.upstreamBindings) do + table.insert(internalData.upstreamConnections, Binding.subscribe(binding, upstreamCallback)) + end end local disconnect = internalData.changeSignal:subscribe(handler) @@ -124,9 +164,8 @@ function Binding.subscribe(binding, handler) If our subscribers count drops to 0, we can safely unsubscribe from our source binding ]] - if internalData.subscriberCount == 0 and internalData.upstreamDisconnect ~= nil then - internalData.upstreamDisconnect() - internalData.upstreamDisconnect = nil + if internalData.subscriberCount == 0 then + binding:__upstreamDisconnect() end end end @@ -145,8 +184,10 @@ function Binding.create(initialValue) subscriberCount = 0, valueTransform = identity, - upstreamBinding = nil, - upstreamDisconnect = nil, + isJoinedBinding = false, + upstreamBindings = {}, + upstreamConnections = {}, + upstreamBindingCount = 0, }, } @@ -165,24 +206,14 @@ end be exposed to users of Roact. ]] function Binding.join(bindings) - local joinedBinding, setter = Binding.create(mapBindingsToValues(bindings)) + local joinedBinding = Binding.create(mapBindingsToValues(bindings)) local internalData = joinedBinding[InternalData] - local upstreamConnections = {} - internalData.upstreamConnections = upstreamConnections - - internalData.upstreamDisconnect = function() - for _, disconnect in pairs(upstreamConnections) do - disconnect() - end - end - - local function updateBinding() - setter(mapBindingsToValues(bindings)) - end + internalData.isJoinedBinding = true for key, binding in pairs(bindings) do - upstreamConnections[key] = Binding.subscribe(binding, updateBinding) + internalData.upstreamBindings[key] = binding + internalData.upstreamBindingCount = internalData.upstreamBindingCount + 1 end return joinedBinding From 0173bb443561ae73d63c53df6bbfe1a406b87a73 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 02:24:40 -0400 Subject: [PATCH 06/13] Fix luacheck warnings --- src/Binding.lua | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Binding.lua b/src/Binding.lua index 6bc59ac9..7fa974ee 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -69,7 +69,7 @@ function bindingPrototype:map(valueTransform) local internalData = binding[InternalData] internalData.valueTransform = valueTransform - + internalData.upstreamBindings.source = self internalData.upstreamBindingCount = internalData.upstreamBindingCount + 1 @@ -137,8 +137,8 @@ function Binding.subscribe(binding, handler) Binding.update(binding, binding:__getValueFromUpstreamBindings()) end - for _, binding in pairs(internalData.upstreamBindings) do - table.insert(internalData.upstreamConnections, Binding.subscribe(binding, upstreamCallback)) + for _, upstreamBinding in pairs(internalData.upstreamBindings) do + table.insert(internalData.upstreamConnections, Binding.subscribe(upstreamBinding, upstreamCallback)) end end From 2d4473d400599d8a21c2d7e394cf8c1970f9e686 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 10:28:56 -0400 Subject: [PATCH 07/13] Type checking --- src/Binding.lua | 11 +++++++++++ src/Binding.spec.lua | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/Binding.lua b/src/Binding.lua index 7fa974ee..d09f29b6 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -206,6 +206,17 @@ end be exposed to users of Roact. ]] function Binding.join(bindings) + if config.typeChecks then + assert(typeof(bindings) == "table", "Bad arg #1 to Binding.join: expected table") + + for key, binding in pairs(bindings) do + --[[ + Bindings exclusively and always have a InternalData member + ]] + assert(binding[InternalData], ("Non-binding value passed into Binding.join at index %q"):format(key)) + end + end + local joinedBinding = Binding.create(mapBindingsToValues(bindings)) local internalData = joinedBinding[InternalData] diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index be03f329..bd8075ac 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -60,6 +60,23 @@ return function() expect(bindingValue[1]).to.equal(3) expect(bindingValue[2]).to.equal(4) end) + + it("should throw when a non-table value is passed", function() + expect(function() + Roact.joinBindings("hi") + end).to.throw() + end) + + it("should throw when a non-binding value is passed via table", function() + expect(function() + local binding = Roact.createBinding(123) + + Roact.joinBindings({ + binding, + "abcde", + }) + end).to.throw() + end) end) describe("Binding object", function() From 4d8f9254889599f690333aa8baad8c78e3e6ab18 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 10:31:04 -0400 Subject: [PATCH 08/13] Check if upstreamBindings is empty instead of checking length --- src/Binding.lua | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Binding.lua b/src/Binding.lua index d09f29b6..041281bc 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -11,6 +11,13 @@ local function identity(value) return value end +--[[ + Determines if a table (array or dictionary) is empty +]] +local function isTableEmpty(value) + return not next(value) +end + --[[ Maps a table of bindings to their respective values. Used in Binding.join. ]] @@ -50,7 +57,7 @@ function bindingPrototype:getValue() This allows us to avoid subscribing to our source until someone has subscribed to us, and avoid creating dangling connections. ]] - if internalData.upstreamBindingCount > 0 then + if not isTableEmpty(internalData.upstreamBindings) then return internalData.valueTransform(self:__getValueFromUpstreamBindings()) end @@ -66,12 +73,9 @@ function bindingPrototype:map(valueTransform) end local binding = Binding.create(valueTransform(self:getValue())) - local internalData = binding[InternalData] - - internalData.valueTransform = valueTransform - internalData.upstreamBindings.source = self - internalData.upstreamBindingCount = internalData.upstreamBindingCount + 1 + binding[InternalData].valueTransform = valueTransform + binding[InternalData].upstreamBindings.source = self return binding end @@ -132,7 +136,7 @@ function Binding.subscribe(binding, handler) we need to create subscriptions to our source bindings so that updates get passed along to us ]] - if internalData.upstreamBindingCount > 0 and internalData.subscriberCount == 0 then + if not isTableEmpty(internalData.upstreamBindings) and internalData.subscriberCount == 0 then local function upstreamCallback() Binding.update(binding, binding:__getValueFromUpstreamBindings()) end @@ -187,7 +191,6 @@ function Binding.create(initialValue) isJoinedBinding = false, upstreamBindings = {}, upstreamConnections = {}, - upstreamBindingCount = 0, }, } @@ -224,7 +227,6 @@ function Binding.join(bindings) for key, binding in pairs(bindings) do internalData.upstreamBindings[key] = binding - internalData.upstreamBindingCount = internalData.upstreamBindingCount + 1 end return joinedBinding From fdf90593c48a6eb31f445090ca46cc682d042413 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 10:32:35 -0400 Subject: [PATCH 09/13] Switch to ipairs, discard upstreamConnections array on disconnect --- src/Binding.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Binding.lua b/src/Binding.lua index 041281bc..5c82af82 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -108,9 +108,11 @@ end function bindingPrototype:__upstreamDisconnect() local internalData = self[InternalData] - for _, disconnect in pairs(internalData.upstreamConnections) do + for _, disconnect in ipairs(internalData.upstreamConnections) do disconnect() end + + internalData.upstreamConnections = {} end --[[ From 867b1ee9630e36d695cf0ec1c9c6a1cd7bce1bcf Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 10:39:03 -0400 Subject: [PATCH 10/13] Move getValue into its own test --- src/Binding.spec.lua | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index bd8075ac..b90938a4 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -55,12 +55,30 @@ return function() update2(4) expect(spy.callCount).to.equal(2) - local bindingValue = joinedBinding:getValue() + local bindingValue = spy.values[1] expect(bindingValue).to.be.a("table") expect(bindingValue[1]).to.equal(3) expect(bindingValue[2]).to.equal(4) end) + it("should return correct values when there are no subscriptions", function() + local binding1, update1 = Binding.create(1) + local binding2, update2 = Binding.create(2) + + local joinedBinding = Binding.join({ + binding1, + binding2, + }) + + update1("foo") + update2("bar") + + local bindingValue = joinedBinding:getValue() + expect(bindingValue).to.be.a("table") + expect(bindingValue[1]).to.equal("foo") + expect(bindingValue[2]).to.equal("bar") + end) + it("should throw when a non-table value is passed", function() expect(function() Roact.joinBindings("hi") From f6f2857d602d66aa71464793fb70a0033d746bfe Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 10:42:09 -0400 Subject: [PATCH 11/13] Switch to Binding.join instead of Roact.joinBindings in tests (whoops!) --- src/Binding.spec.lua | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Binding.spec.lua b/src/Binding.spec.lua index b90938a4..d0360b21 100644 --- a/src/Binding.spec.lua +++ b/src/Binding.spec.lua @@ -81,15 +81,15 @@ return function() it("should throw when a non-table value is passed", function() expect(function() - Roact.joinBindings("hi") + Binding.join("hi") end).to.throw() end) it("should throw when a non-binding value is passed via table", function() expect(function() - local binding = Roact.createBinding(123) + local binding = Binding.create(123) - Roact.joinBindings({ + Binding.join({ binding, "abcde", }) From 5acfc21134cb5374bfaa4626fe6e76e9861c1235 Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 16:39:04 -0400 Subject: [PATCH 12/13] Use Type.of for type checking instead of InternalData --- src/Binding.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Binding.lua b/src/Binding.lua index 5c82af82..16f3ecc6 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -218,7 +218,7 @@ function Binding.join(bindings) --[[ Bindings exclusively and always have a InternalData member ]] - assert(binding[InternalData], ("Non-binding value passed into Binding.join at index %q"):format(key)) + assert(Type.of(binding) == Type.Binding, ("Non-binding value passed into Binding.join at index %q"):format(key)) end end From a6eaf1265132363d706b63bda813119cb7a2c6fd Mon Sep 17 00:00:00 2001 From: Reselim Date: Sun, 5 May 2019 21:53:09 -0400 Subject: [PATCH 13/13] Remove comment in type check --- src/Binding.lua | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Binding.lua b/src/Binding.lua index 16f3ecc6..04ce1d0b 100644 --- a/src/Binding.lua +++ b/src/Binding.lua @@ -215,9 +215,6 @@ function Binding.join(bindings) assert(typeof(bindings) == "table", "Bad arg #1 to Binding.join: expected table") for key, binding in pairs(bindings) do - --[[ - Bindings exclusively and always have a InternalData member - ]] assert(Type.of(binding) == Type.Binding, ("Non-binding value passed into Binding.join at index %q"):format(key)) end end