diff --git a/CHANGELOG.md b/CHANGELOG.md index 16fe6dcc..3daad252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,14 @@ # Roact Changelog ## Unreleased Changes +* Introduce forwardRef ([#307](https://github.com/Roblox/roact/pull/307)). * Fixed a bug where the Roact tree could get into a broken state when processing changes to child instances outside the standard lifecycle. - This change is behind the config value tempFixUpdateChildrenReEntrancy ([#301](https://github.com/Roblox/roact/pull/301)) + * This change is behind the config value tempFixUpdateChildrenReEntrancy ([#301](https://github.com/Roblox/roact/pull/301)) * Added color schemes for documentation based on user preference ([#290](https://github.com/Roblox/roact/pull/290)). * Fixed stack trace level when throwing an error in `createReconciler` ([#297](https://github.com/Roblox/roact/pull/297)). * Optimized the memory usage of 'createSignal' implementation. ([#304](https://github.com/Roblox/roact/pull/304)) -## [1.3.1](https://github.com/Roblox/roact/releases/tag/v1.3.0) (November 19th, 2020) +## [1.3.1](https://github.com/Roblox/roact/releases/tag/v1.3.1) (November 19th, 2020) * Added component name to property validation error message ([#275](https://github.com/Roblox/roact/pull/275)) ## [1.3.0](https://github.com/Roblox/roact/releases/tag/v1.3.0) (May 5th, 2020) diff --git a/docs/advanced/bindings-and-refs.md b/docs/advanced/bindings-and-refs.md index 2ace4202..6df8d720 100644 --- a/docs/advanced/bindings-and-refs.md +++ b/docs/advanced/bindings-and-refs.md @@ -137,6 +137,63 @@ end Since refs use bindings under the hood, they will be automatically updated whenever the ref changes. This means there's no need to worry about the order in which refs are assigned relative to when properties that use them get set. +### Ref Forwarding +In Roact 1.x, refs can only be applied to host components, _not_ stateful or function components. However, stateful or function components may accept a ref in order to pass it along to an underlying host component. In order to implement this, we wrap the given component with `Roact.forwardRef`. + +Suppose we have a styled TextBox component that still needs to accept a ref, so that users of the component can trigger functionality like `TextBox:CaptureFocus()`: + +```lua +local function FancyTextBox(props) + return Roact.createElement("TextBox", { + Multiline = true, + PlaceholderText = "Enter your text here", + PlaceholderColor3 = Color3.new(0.4, 0.4, 0.4), + [Roact.Change.Text] = props.onTextChange, + }) +end +``` + +If we were to create an element using the above component, we'd be unable to get a ref to point to the underlying "TextBox" Instance: + +```lua +local Form = Roact.Component:extend("Form") +function Form:init() + self.textBoxRef = Roact.createRef() +end + +function Form:render() + return React.createElement(FancyTextBox, { + onTextChange = function(value) + print("text value updated to:", value) + end + -- This doesn't actually get assigned to the underlying TextBox! + [Roact.Ref] = self.textBoxRef, + }) +end + +function Form:didMount() + -- Since self.textBoxRef never gets assigned to a host component, this + -- doesn't work, and in fact will be an attempt to access a nil reference! + self.textBoxRef.current:CaptureFocus() +end +``` + +In this instance, `FancyTextBox` simply doesn't do anything with the ref passed into it. However, we can easily update it using forwardRef: + +```lua +local FancyTextBox = React.forwardRef(function(props, ref) + return Roact.createElement("TextBox", { + Multiline = true, + PlaceholderText = "Enter your text here", + PlaceholderColor3 = Color3.new(0.4, 0.4, 0.4), + [Roact.Change.Text] = props.onTextChange, + [Roact.Ref] = ref, + }) +end) +``` + +With the above change, `FancyTextBox` now accepts a ref and assigns it to the "TextBox" host component that it renders under the hood. Our `Form` implementation will successfully capture focus on `didMount`. + ### Function Refs The original ref API was based on functions instead of objects (and does not use bindings). Its use is not recommended for most cases anymore. diff --git a/docs/api-reference.md b/docs/api-reference.md index a5fd2f68..72a07086 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -179,6 +179,15 @@ Creates a new reference object that can be used with [Roact.Ref](#roactref). --- +### Roact.forwardRef +``` +Roact.createRef(render: (props: table, ref: Ref) -> RoactElement) -> RoactComponent +``` + +Creates a new component given a render function that accepts both props and a ref, allowing a ref to be forwarded to an underlying host component via [Roact.Ref](#roactref). + +--- + ### Roact.createContext !!! success "Added in Roact 1.3.0" diff --git a/src/forwardRef.lua b/src/forwardRef.lua new file mode 100644 index 00000000..77fff90d --- /dev/null +++ b/src/forwardRef.lua @@ -0,0 +1,28 @@ +local assign = require(script.Parent.assign) +local None = require(script.Parent.None) +local Ref = require(script.Parent.PropMarkers.Ref) + +local config = require(script.Parent.GlobalConfig).get() + +local excludeRef = { + [Ref] = None, +} + +--[[ + Allows forwarding of refs to underlying host components. Accepts a render + callback which accepts props and a ref, and returns an element. +]] +local function forwardRef(render) + if config.typeChecks then + assert(typeof(render) == "function", "Expected arg #1 to be a function") + end + + return function(props) + local ref = props[Ref] + local propsWithoutRef = assign({}, props, excludeRef) + + return render(propsWithoutRef, ref) + end +end + +return forwardRef \ No newline at end of file diff --git a/src/forwardRef.spec.lua b/src/forwardRef.spec.lua new file mode 100644 index 00000000..6d65101c --- /dev/null +++ b/src/forwardRef.spec.lua @@ -0,0 +1,341 @@ +-- Tests loosely adapted from those found at: +-- * https://github.com/facebook/react/blob/v17.0.1/packages/react/src/__tests__/forwardRef-test.js +-- * https://github.com/facebook/react/blob/v17.0.1/packages/react/src/__tests__/forwardRef-test.internal.js +return function() + local assign = require(script.Parent.assign) + local createElement = require(script.Parent.createElement) + local createRef = require(script.Parent.createRef) + local forwardRef = require(script.Parent.forwardRef) + local createReconciler = require(script.Parent.createReconciler) + local Component = require(script.Parent.Component) + local GlobalConfig = require(script.Parent.GlobalConfig) + local Ref = require(script.Parent.PropMarkers.Ref) + + local RobloxRenderer = require(script.Parent.RobloxRenderer) + + local reconciler = createReconciler(RobloxRenderer) + + it("should update refs when switching between children", function() + local function FunctionComponent(props) + local forwardedRef = props.forwardedRef + local setRefOnDiv = props.setRefOnDiv + -- deviation: clearer to express this way, since we don't have real + -- ternaries + local firstRef, secondRef + if setRefOnDiv then + firstRef = forwardedRef + else + secondRef = forwardedRef + end + return createElement("Frame", nil, { + First = createElement("Frame", { + [Ref] = firstRef + }, { + Child = createElement("TextLabel", { + Text = "First" + }) + }), + Second = createElement("ScrollingFrame", { + [Ref] = secondRef + }, { + Child = createElement("TextLabel", { + Text = "Second" + }) + }) + }) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(FunctionComponent, assign({}, props, { forwardedRef = ref })) + end) + + local ref = createRef() + + local element = createElement(RefForwardingComponent, { + [Ref] = ref, + setRefOnDiv = true, + }) + local tree = reconciler.mountVirtualTree(element, nil, "switch refs") + expect(ref.current.ClassName).to.equal("Frame") + reconciler.unmountVirtualTree(tree) + + element = createElement(RefForwardingComponent, { + [Ref] = ref, + setRefOnDiv = false, + }) + tree = reconciler.mountVirtualTree(element, nil, "switch refs") + expect(ref.current.ClassName).to.equal("ScrollingFrame") + reconciler.unmountVirtualTree(tree) + end) + + it("should support rendering nil", function() + local RefForwardingComponent = forwardRef(function(props, ref) + return nil + end) + + local ref = createRef() + + local element = createElement(RefForwardingComponent, { [Ref] = ref }) + local tree = reconciler.mountVirtualTree(element, nil, "nil ref") + expect(ref.current).to.equal(nil) + reconciler.unmountVirtualTree(tree) + end) + + it("should support rendering nil for multiple children", function() + local RefForwardingComponent = forwardRef(function(props, ref) + return nil + end) + + local ref = createRef() + + local element = createElement("Frame", nil, { + NoRef1 = createElement("Frame"), + WithRef = createElement(RefForwardingComponent, { [Ref] = ref }), + NoRef2 = createElement("Frame"), + }) + local tree = reconciler.mountVirtualTree(element, nil, "multiple children nil ref") + expect(ref.current).to.equal(nil) + reconciler.unmountVirtualTree(tree) + end) + + -- We could support this by having forwardRef return a stateful component, + -- but it's likely not necessary + itSKIP("should support defaultProps", function() + local function FunctionComponent(props) + local forwardedRef = props.forwardedRef + local optional = props.optional + local required = props.required + return createElement("Frame", { + [Ref] = forwardedRef, + }, { + OptionalChild = optional, + RequiredChild = required, + }) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(FunctionComponent, assign({}, props, { + forwardedRef = ref + })) + end) + RefForwardingComponent.defaultProps = { + optional = createElement("TextLabel"), + } + + local ref = createRef() + + local element = createElement(RefForwardingComponent, { + [Ref] = ref, + optional = createElement("Frame"), + required = createElement("ScrollingFrame"), + }) + + local tree = reconciler.mountVirtualTree(element, nil, "with optional") + + expect(ref.current:FindFirstChild("OptionalChild").ClassName).to.equal("Frame") + expect(ref.current:FindFirstChild("RequiredChild").ClassName).to.equal("ScrollingFrame") + + reconciler.unmountVirtualTree(tree) + element = createElement(RefForwardingComponent, { + [Ref] = ref, + required = createElement("ScrollingFrame"), + }) + tree = reconciler.mountVirtualTree(element, nil, "with default") + + expect(ref.current:FindFirstChild("OptionalChild").ClassName).to.equal("TextLabel") + expect(ref.current:FindFirstChild("RequiredChild").ClassName).to.equal("ScrollingFrame") + reconciler.unmountVirtualTree(tree) + end) + + it("should error if not provided a callback when type checking is enabled", function() + GlobalConfig.scoped({ + typeChecks = true, + }, function() + expect(function() + forwardRef(nil) + end).to.throw() + end) + + GlobalConfig.scoped({ + typeChecks = true, + }, function() + expect(function() + forwardRef("foo") + end).to.throw() + end) + end) + + it("should work without a ref to be forwarded", function() + local function Child() + return nil + end + + local function Wrapper(props) + return createElement(Child, assign({}, props, { [Ref] = props.forwardedRef })) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(Wrapper, assign({}, props, { forwardedRef = ref })) + end) + + local element = createElement(RefForwardingComponent, { value = 123 }) + local tree = reconciler.mountVirtualTree(element, nil, "nil ref") + reconciler.unmountVirtualTree(tree) + end) + + it("should forward a ref for a single child", function() + local value + local function Child(props) + value = props.value + return createElement("Frame", { + [Ref] = props[Ref] + }) + end + + local function Wrapper(props) + return createElement(Child, assign({}, props, { [Ref] = props.forwardedRef })) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(Wrapper, assign({}, props, { forwardedRef = ref })) + end) + + local ref = createRef() + + local element = createElement(RefForwardingComponent, { [Ref] = ref, value = 123 }) + local tree = reconciler.mountVirtualTree(element, nil, "single child ref") + expect(value).to.equal(123) + expect(ref.current.ClassName).to.equal("Frame") + reconciler.unmountVirtualTree(tree) + end) + + it("should forward a ref for multiple children", function() + local function Child(props) + return createElement("Frame", { + [Ref] = props[Ref] + }) + end + + local function Wrapper(props) + return createElement(Child, assign({}, props, { [Ref] = props.forwardedRef })) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(Wrapper, assign({}, props, { forwardedRef = ref })) + end) + + local ref = createRef() + + local element = createElement("Frame", nil, { + NoRef1 = createElement("Frame"), + WithRef = createElement(RefForwardingComponent, { [Ref] = ref }), + NoRef2 = createElement("Frame"), + }) + local tree = reconciler.mountVirtualTree(element, nil, "multi child ref") + expect(ref.current.ClassName).to.equal("Frame") + reconciler.unmountVirtualTree(tree) + end) + + it("should maintain child instance and ref through updates", function() + local value + local function Child(props) + value = props.value + return createElement("Frame", { + [Ref] = props[Ref] + }) + end + + local function Wrapper(props) + return createElement(Child, assign({}, props, { [Ref] = props.forwardedRef })) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(Wrapper, assign({}, props, { forwardedRef = ref })) + end) + + local setRefCount = 0 + local refValue + + local setRef = function(r) + setRefCount = setRefCount + 1 + refValue = r + end + + local element = createElement(RefForwardingComponent, { [Ref] = setRef, value = 123 }) + local tree = reconciler.mountVirtualTree(element, nil, "maintains instance") + + expect(value).to.equal(123) + expect(refValue.ClassName).to.equal("Frame") + expect(setRefCount).to.equal(1) + + element = createElement(RefForwardingComponent, { [Ref] = setRef, value = 456 }) + tree = reconciler.updateVirtualTree(tree, element) + + expect(value).to.equal(456) + expect(setRefCount).to.equal(1) + reconciler.unmountVirtualTree(tree) + end) + + it("should not re-run the render callback on a deep setState", function() + local inst + local renders = {} + + local Inner = Component:extend("Inner") + function Inner:render() + table.insert(renders, "Inner") + inst = self + return createElement("Frame", { [Ref] = self.props.forwardedRef }) + end + + local function Middle(props) + table.insert(renders, "Middle") + return createElement(Inner, props) + end + + local Forward = forwardRef(function(props, ref) + table.insert(renders, "Forward") + return createElement(Middle, assign({}, props, { forwardedRef = ref })) + end) + + local function App() + table.insert(renders, "App") + return createElement(Forward) + end + + local tree = reconciler.mountVirtualTree(createElement(App), nil, "deep setState") + expect(#renders).to.equal(4) + expect(renders[1]).to.equal("App") + expect(renders[2]).to.equal("Forward") + expect(renders[3]).to.equal("Middle") + expect(renders[4]).to.equal("Inner") + + renders = {} + inst:setState({}) + expect(#renders).to.equal(1) + expect(renders[1]).to.equal("Inner") + reconciler.unmountVirtualTree(tree) + end) + + it("should not include the ref in the forwarded props", function() + local capturedProps + local function CaptureProps(props) + capturedProps = props + return createElement("Frame", { [Ref] = props.forwardedRef }) + end + + local RefForwardingComponent = forwardRef(function(props, ref) + return createElement(CaptureProps, assign({}, props, { forwardedRef = ref })) + end) + + local ref = createRef() + local element = createElement(RefForwardingComponent, { + [Ref] = ref, + }) + + local tree = reconciler.mountVirtualTree(element, nil, "no ref in props") + expect(capturedProps).to.be.ok() + expect(capturedProps.forwardedRef).to.equal(ref) + expect(capturedProps[Ref]).to.equal(nil) + reconciler.unmountVirtualTree(tree) + end) +end diff --git a/src/init.lua b/src/init.lua index 0bac3010..1696d235 100644 --- a/src/init.lua +++ b/src/init.lua @@ -21,6 +21,7 @@ local Roact = strict { None = require(script.None), Portal = require(script.Portal), createRef = require(script.createRef), + forwardRef = require(script.forwardRef), createBinding = Binding.create, joinBindings = Binding.join, createContext = require(script.createContext), diff --git a/src/init.spec.lua b/src/init.spec.lua index 652ee19a..23fef065 100644 --- a/src/init.spec.lua +++ b/src/init.spec.lua @@ -6,6 +6,7 @@ return function() createElement = "function", createFragment = "function", createRef = "function", + forwardRef = "function", createBinding = "function", joinBindings = "function", mount = "function",