Refactor bindings, introducing joinBindings

src/Binding.lua

@@ -4,146 +4,156 @@ local Type = require(script.Parent.Type)
 
 local config = require(script.Parent.GlobalConfig).get()
 
---[[
-	Default mapping function used for non-mapped bindings
-]]
-local function identity(value)
-	return value
+local BindingImpl = Symbol.named("BindingImpl")
+
+local BindingInternalApi = {}
+
+local BindingPublicMethods = {}
+
+function BindingPublicMethods:getValue()
+	return BindingInternalApi.getValue(self)
 end
 
-local Binding = {}
+function BindingPublicMethods:map(predicate)
+	return BindingInternalApi.map(self, predicate)
+end
 
---[[
-	Set of keys for fields that are internal to Bindings
-]]
-local InternalData = Symbol.named("InternalData")
+local BindingPublicMeta = {
+	__index = BindingPublicMethods,
+	__tostring = function(self)
+		return string.format("RoactBinding(%s)", tostring(self:getValue()))
+	end,
+}
 
-local bindingPrototype = {}
-bindingPrototype.__index = bindingPrototype
-bindingPrototype.__tostring = function(self)
-	return ("RoactBinding(%s)"):format(tostring(self[InternalData].value))
+function BindingInternalApi.update(binding, newValue)
+	return binding[BindingImpl].update(newValue)
 end
 
---[[
-	Get the current value from a binding
-]]
-function bindingPrototype:getValue()
-	local internalData = self[InternalData]
-
-	--[[
-		If our source is another binding but we're not subscribed, we'll
-		return the mapped value from our upstream binding.
-
-		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())
-	end
+function BindingInternalApi.subscribe(binding, callback)
+	return binding[BindingImpl].subscribe(callback)
+end
 
-	return internalData.value
+function BindingInternalApi.getValue(binding)
+	return binding[BindingImpl].getValue()
 end
 
---[[
-	Creates a new binding from this one with the given mapping.
-]]
-function bindingPrototype:map(valueTransform)
-	if config.typeChecks then
-		assert(typeof(valueTransform) == "function", "Bad arg #1 to binding:map: expected function")
+function BindingInternalApi.create(initialValue)
+	local impl = {
+		value = initialValue,
+		changeSignal = createSignal(),
+	}
+
+	function impl.subscribe(callback)
+		return impl.changeSignal:subscribe(callback)
 	end
 
-	local binding = Binding.create(valueTransform(self:getValue()))
+	function impl.update(newValue)
+		impl.value = newValue
+		impl.changeSignal:fire(newValue)
+	end
 
-	binding[InternalData].valueTransform = valueTransform
-	binding[InternalData].upstreamBinding = self
+	function impl.getValue()
+		return impl.value
+	end
 
-	return binding
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta), impl.update
 end
 
---[[
-	Update a binding's value. This is only accessible by Roact.
-]]
-function Binding.update(binding, newValue)
-	local internalData = binding[InternalData]
+function BindingInternalApi.map(upstreamBinding, predicate)
+	if config.typeChecks then
+		assert(Type.of(upstreamBinding) == Type.Binding, "Expected arg #1 to be a binding")
+		assert(typeof(predicate) == "function", "Expected arg #1 to be a function")
+	end
 
-	newValue = internalData.valueTransform(newValue)
+	local impl = {}
 
-	internalData.value = newValue
-	internalData.changeSignal:fire(newValue)
-end
-
---[[
-	Subscribe to a binding's change signal. This is only accessible by Roact.
-]]
-function Binding.subscribe(binding, handler)
-	local internalData = binding[InternalData]
-
-	--[[
-		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
-		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)
+	function impl.subscribe(callback)
+		return BindingInternalApi.subscribe(upstreamBinding, function(newValue)
+			callback(predicate(newValue))
 		end)
 	end
 
-	local disconnect = internalData.changeSignal:subscribe(handler)
-	internalData.subscriberCount = internalData.subscriberCount + 1
+	function impl.update(newValue)
+		-- This operation doesn't make sense; we can leave it as a no-op
+	end
 
-	local disconnected = false
+	function impl.getValue()
+		return predicate(upstreamBinding:getValue())
+	end
 
-	--[[
-		We wrap the disconnect function so that we can manage our subscriptions
-		when the disconnect is triggered
-	]]
-	return function()
-		if disconnected then
-			return
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta)
+end
+
+function BindingInternalApi.join(upstreamBindings)
+	if config.typeChecks then
+		assert(typeof(upstreamBindings) == "table", "Expected arg #1 to be of type table")
+
+		for key, value in pairs(upstreamBindings) do
+			if Type.of(value) ~= Type.Binding then
+				local message = (
+					"Expected arg #1 to contain only bindings, but key %s had a non-binding value"
+				):format(
+					tostring(key)
+				)
+				error(message, 2)
+			end
 		end
+	end
 
-		disconnected = true
-		disconnect()
-		internalData.subscriberCount = internalData.subscriberCount - 1
-
-		--[[
-			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
+	local impl = {
+		disconnectMethods = nil,
+	}
+
+	local function getValue()
+		local value = {}
+
+		for key, upstream in pairs(upstreamBindings) do
+			value[key] = upstream:getValue()
 		end
+
+		return value
 	end
-end
 
---[[
-	Create a new binding object with the given starting value. This
-	function will be exposed to users of Roact.
-]]
-function Binding.create(initialValue)
-	local binding = {
-		[Type] = Type.Binding,
+	function impl.subscribe(callback)
+		local disconnects = {}
 
-		[InternalData] = {
-			value = initialValue,
-			changeSignal = createSignal(),
-			subscriberCount = 0,
+		for key, upstream in pairs(upstreamBindings) do
+			disconnects[key] = BindingInternalApi.subscribe(upstream, function(newValue)
+				callback(getValue())
+			end)
+		end
 
-			valueTransform = identity,
-			upstreamBinding = nil,
-			upstreamDisconnect = nil,
-		},
-	}
+		return function()
+			if disconnects == nil then
+				return
+			end
 
-	setmetatable(binding, bindingPrototype)
+			for _, disconnect in pairs(disconnects) do
+				disconnect()
+			end
 
-	local setter = function(newValue)
-		Binding.update(binding, newValue)
+			disconnects = nil
+		end
 	end
 
-	return binding, setter
+	function impl.update(newValue)
+		-- This operation doesn't make sense; we can leave it as a no-op
+	end
+
+	function impl.getValue()
+		return getValue()
+	end
+
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta)
 end
 
-return Binding
+return BindingInternalApi

src/Binding.spec.lua

@@ -119,4 +119,76 @@ return function()
 			expect(lengthSpy.callCount).to.equal(1)
 		end)
 	end)
-end
+
+	describe("Binding.join", function()
+		it("should have getValue", function()
+			local binding1 = Binding.create(1)
+			local binding2 = Binding.create(2)
+			local binding3 = Binding.create(3)
+
+			local joinedBinding = Binding.join({
+				binding1,
+				binding2,
+				foo = binding3,
+			})
+
+			local bindingValue = joinedBinding:getValue()
+			expect(bindingValue).to.be.a("table")
+			expect(bindingValue[1]).to.equal(1)
+			expect(bindingValue[2]).to.equal(2)
+			expect(bindingValue.foo).to.equal(3)
+		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 binding3, update3 = Binding.create(3)
+
+			local joinedBinding = Binding.join({
+				binding1,
+				binding2,
+				foo = binding3,
+			})
+
+			local spy = createSpy()
+			Binding.subscribe(joinedBinding, spy.value)
+
+			expect(spy.callCount).to.equal(0)
+
+			update1(3)
+			expect(spy.callCount).to.equal(1)
+
+			local args = spy:captureValues("value")
+			expect(args.value).to.be.a("table")
+			expect(args.value[1]).to.equal(3)
+			expect(args.value[2]).to.equal(2)
+			expect(args.value["foo"]).to.equal(3)
+
+			update2(4)
+			expect(spy.callCount).to.equal(2)
+
+			args = spy:captureValues("value")
+			expect(args.value).to.be.a("table")
+			expect(args.value[1]).to.equal(3)
+			expect(args.value[2]).to.equal(4)
+			expect(args.value["foo"]).to.equal(3)
+		end)
+
+		it("should throw when a non-table value is passed", function()
+			expect(function()
+				Binding.join("hi")
+			end).to.throw()
+		end)
+
+		it("should throw when a non-binding value is passed via table", function()
+			expect(function()
+				local binding = Binding.create(123)
+
+				Binding.join({
+					binding,
+					"abcde",
+				})
+			end).to.throw()
+		end)
+	end)
+end

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),

src/init.spec.lua

@@ -7,6 +7,7 @@ return function()
 			createFragment = "function",
 			createRef = "function",
 			createBinding = "function",
+			joinBindings = "function",
 			mount = "function",
 			unmount = "function",
 			update = "function",