Refactor bindings, add joinBindings (#208)

Refactored bindings to make them adhere to a common interface instead of having an implicit kind based on combinations of variables being nil!

Implemented joinBindings in this new paradigm, which became much simpler.

CHANGELOG.md

@@ -1,8 +1,8 @@
 # Roact Changelog
 
-## Unreleased
-
+## Unreleased Changes
 * Fixed an issue where updating a host element with children to an element with `nil` children caused the old children to not be unmounted. ([#210](https://github.com/Roblox/roact/pull/210))
+* Added `Roact.joinBindings`, which allows combining multiple bindings into a single binding that can be mapped. ([#208](https://github.com/Roblox/roact/pull/208))
 
 ## [1.0.0](https://github.com/Roblox/roact/releases/tag/v1.0.0)
 This release significantly reworks Roact internals to enable new features and optimizations.

docs/api-reference.md

@@ -120,6 +120,56 @@ Returns a new binding that maps the existing binding's value to something else.
 
 ---
 
+### Roact.joinBindings
+<div class="api-addition">Unreleased API</div>
+
+```
+Roact.joinBindings(bindings) -> Binding
+where
+	bindings: { [any]: Binding }
+```
+
+Combines multiple bindings into a single binding. The new binding's value will have the same keys as the input table of bindings.
+
+`joinBindings` is usually used alongside `Binding:map`:
+
+```lua
+local function Flex()
+	local aSize, setASize = Roact.createBinding(Vector2.new())
+	local bSize, setBSize = Roact.createBinding(Vector2.new())
+
+	return Roact.createElement("Frame", {
+		Size = Roact.joinBindings({aSize, bSize}):map(function(sizes)
+			local sum = Vector2.new()
+
+			for _, size in ipairs(sizes) do
+				sum = sum + size
+			end
+
+			return UDim2.new(0, sum.X,  0, sum.Y)
+		end),
+	}, {
+		A = Roact.createElement("Frame", {
+			Size = UDim2.new(1, 0, 0, 30),
+			[Roact.Change.AbsoluteSize] = function(instance)
+				setASize(instance.Size)
+			end,
+		}),
+		B = Roact.createElement("Frame", {
+			Size = UDim2.new(1, 0, 0, 30),
+			Position = aSize:map(function(size)
+				return UDim2.new(0, 0, 0, size.Y)
+			end),
+			[Roact.Change.AbsoluteSize] = function(instance)
+				setBSize(instance.Size)
+			end,
+		}),
+	})
+end
+```
+
+---
+
 ### Roact.createRef
 ```
 Roact.createRef() -> Ref

src/Binding.lua

@@ -4,146 +4,154 @@ 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 bindingPrototype = {}
+
+function bindingPrototype:getValue()
+	return BindingInternalApi.getValue(self)
 end
 
-local Binding = {}
+function bindingPrototype: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 = bindingPrototype,
+	__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]
+function BindingInternalApi.subscribe(binding, callback)
+	return binding[BindingImpl].subscribe(callback)
+end
 
-	--[[
-		If our source is another binding but we're not subscribed, we'll
-		return the mapped value from our upstream binding.
+function BindingInternalApi.getValue(binding)
+	return binding[BindingImpl].getValue()
+end
 
-		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())
+function BindingInternalApi.create(initialValue)
+	local impl = {
+		value = initialValue,
+		changeSignal = createSignal(),
+	}
+
+	function impl.subscribe(callback)
+		return impl.changeSignal:subscribe(callback)
 	end
 
-	return internalData.value
+	function impl.update(newValue)
+		impl.value = newValue
+		impl.changeSignal:fire(newValue)
+	end
+
+	function impl.getValue()
+		return impl.value
+	end
+
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta), impl.update
 end
 
---[[
-	Creates a new binding from this one with the given mapping.
-]]
-function bindingPrototype:map(valueTransform)
+function BindingInternalApi.map(upstreamBinding, predicate)
 	if config.typeChecks then
-		assert(typeof(valueTransform) == "function", "Bad arg #1 to binding:map: expected function")
+		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
 
-	local binding = Binding.create(valueTransform(self:getValue()))
+	local impl = {}
 
-	binding[InternalData].valueTransform = valueTransform
-	binding[InternalData].upstreamBinding = self
-
-	return binding
-end
+	function impl.subscribe(callback)
+		return BindingInternalApi.subscribe(upstreamBinding, function(newValue)
+			callback(predicate(newValue))
+		end)
+	end
 
---[[
-	Update a binding's value. This is only accessible by Roact.
-]]
-function Binding.update(binding, newValue)
-	local internalData = binding[InternalData]
+	function impl.update(newValue)
+		error("Bindings created by Binding:map(fn) cannot be updated directly", 2)
+	end
 
-	newValue = internalData.valueTransform(newValue)
+	function impl.getValue()
+		return predicate(upstreamBinding:getValue())
+	end
 
-	internalData.value = newValue
-	internalData.changeSignal:fire(newValue)
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta)
 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)
-		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 %q had a non-binding value"
+				):format(
+					tostring(key)
+				)
+				error(message, 2)
+			end
+		end
 	end
 
-	local disconnect = internalData.changeSignal:subscribe(handler)
-	internalData.subscriberCount = internalData.subscriberCount + 1
+	local impl = {}
 
-	local disconnected = false
+	local function getValue()
+		local value = {}
 
-	--[[
-		We wrap the disconnect function so that we can manage our subscriptions
-		when the disconnect is triggered
-	]]
-	return function()
-		if disconnected then
-			return
+		for key, upstream in pairs(upstreamBindings) do
+			value[key] = upstream:getValue()
 		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
-		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)
+		error("Bindings created by joinBindings(...) cannot be updated directly", 2)
+	end
+
+	function impl.getValue()
+		return getValue()
+	end
+
+	return setmetatable({
+		[Type] = Type.Binding,
+		[BindingImpl] = impl,
+	}, BindingPublicMeta)
 end
 
-return Binding
+return BindingInternalApi