createRef API #92

AmaranthineCodices · 2018-05-15T12:19:32Z

Closes #70.

Adds a createRef function to the global Roact namespace. createRef returns an object reference with a current property that can be used like existing functional refs. Example usage:

local SomeTextbox = Roact.Component:extend("SomeTextbox")

function SomeTextbox:init()
    self.textboxRef = Roact.createRef()
end

function SomeTextbox:render()
    return Roact.createElement("TextBox", {
        -- ...
        [Roact.Ref] = self.textboxRef
    })
end

function SomeTextbox:didMount()
    self.textboxRef.current:CaptureFocus()
end

coveralls · 2018-05-15T12:23:34Z

Coverage increased (+0.4%) to 90.24% when pulling b139cc3 on AmaranthineCodices:createRef into 93358ca on Roblox:master.

LPGhatguy

Hooray!

Minor actual code changes, mostly just want to push some of the docs into a new PR after this one!

LPGhatguy · 2018-05-15T17:34:11Z

docs/api-reference.md

@@ -72,6 +79,10 @@ If you're writing a new functional or stateful element that needs to be used lik
 Use `Roact.Ref` as a key into the props of a primitive element to receive a handle to the underlying Roblox Instance.

 ```lua
+Roact.createElement("Frame", {
+	[Roact.Ref] = objectReference,
+})


This example seems sort of incomplete -- we should break the functional and object ref examples into two separate blocks and label what they do!

LPGhatguy · 2018-05-15T17:35:38Z

docs/advanced/refs.md

+When a user clicks on the outer `ImageButton`, the `captureFocus` method will be called and the `TextBox` instance will get focus as if it had been clicked on directly.
+
+## Functional Refs
+Roact allows you to use functions as refs. The function will be called with the Roblox object that Roact creates. For example, this is the SearchBar component from above, modified to use functional refs instead of object refs:


Can we move the guie changes to a new PR? There are quite a few more things we need to catch across the docs, and I want to let the API simmer a bit before recommending it as the new default.

Fair enough, done!

LPGhatguy · 2018-05-15T18:04:48Z

lib/Reconciler.lua

+		local ref = element.props[Core.Ref]
+
+		if ref then
+			applyRef(ref, nil)


Let's move this condition into applyRef itself, which would make all of these invocations a bit cleaner!

That'd help clean some stuff up, yeah!

This reverts commit e2f452f.

LPGhatguy · 2018-05-16T00:57:05Z

lib/Reconciler.lua

+	Correctly handles both function-style and object-style refs.
+]]
+local function applyRef(ref, newRbx)
+	if not ref then return end


Let's explicitly check if ref == nil and then put the return statement on its own line.

LPGhatguy · 2018-05-16T00:58:03Z

lib/createRef.spec.lua

+	it("should support tostring on refs", function()
+		local ref = createRef()
+		expect(tostring(ref)).to.equal("RoactReference(\"nil\")")
+	end)


If we're going to test tostring on these, let's also verify that they take on the name of their current value if set, too!

That's fair, done!

LPGhatguy · 2018-05-16T01:04:58Z

lib/createRef.lua

+
+local refMetatable = {
+	__tostring = function(self)
+		return ("RoactReference(%q)"):format(tostring(self.current))


Thinking about it now, do you think we should just format these with %s instead of %q? Since they'll mostly be Roblox objects, I don't know if it makes much sense to quote them!

That's true - I'm used to quoting everything, but in this case it doesn't do much good! Fixed now.

LPGhatguy · 2018-05-16T19:12:41Z

lib/Reconciler.lua

@@ -323,7 +335,7 @@ function Reconciler._reconcileInternal(instanceHandle, newElement)

 		-- Cancel the old ref before we make changes. Apply the new one after.
 		if refChanged and oldRef then
-			oldRef(nil)
+			applyRef(oldRef, nil)


We need to update the branch on line 349 that applies newRef as well! It's calling newRef as a function.

LPGhatguy · 2018-05-16T19:13:09Z

lib/Reconciler.spec.lua

+		expect(currentRbx).to.be.ok()
+		Reconciler.unmount(handle)
+		expect(currentRbx).to.never.be.ok()
+	end)


Can we add a test that makes sure that refs can be unattached and attached through Reconciler.reconcile?

AmaranthineCodices added 5 commits May 14, 2018 21:40

add createRef function

4e773ef

handle correctly in reconciler

82b2330

basic test case for createRef

72b907c

test cases for references

35a9df7

export createRef

d002373

AmaranthineCodices added 2 commits May 15, 2018 10:19

clean up ref-related API docs

e2f452f

add a tostring metamethod to refs for debugging purposes

b624801

AmaranthineCodices mentioned this pull request May 15, 2018

validateProps #94

Closed

2 tasks

LPGhatguy suggested changes May 15, 2018

View reviewed changes

AmaranthineCodices added 4 commits May 15, 2018 17:53

Revert "clean up ref-related API docs"

9ce1d84

This reverts commit e2f452f.

refactor applyRef to eliminate some nil checks

3cda6f7

re-add API docs

1f86eb1

descriptive comments!

878ecc5

LPGhatguy reviewed May 16, 2018

View reviewed changes

AmaranthineCodices added 2 commits May 15, 2018 21:00

fix applyRef style

598af40

expand tostring test case

6f770ab

LPGhatguy reviewed May 16, 2018

View reviewed changes

AmaranthineCodices added 2 commits May 15, 2018 21:07

don't quote strings

8c6cb36

fix tests, oops!

4614af4

LPGhatguy suggested changes May 16, 2018

View reviewed changes

AmaranthineCodices added 4 commits May 16, 2018 17:33

fix missing ref!

3378d4a

fix changing refs

336db31

test cases for changing refs

9de73c7

add to CHANGELOG

b139cc3

LPGhatguy approved these changes May 16, 2018

View reviewed changes

SlartibartfastFjords approved these changes May 16, 2018

View reviewed changes

LPGhatguy merged commit b25a063 into Roblox:master May 16, 2018

LPGhatguy mentioned this pull request May 21, 2018

Change refs in one pass, making switching refs atomic #98

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

createRef API #92

createRef API #92

AmaranthineCodices commented May 15, 2018

coveralls commented May 15, 2018 •

edited

Loading

LPGhatguy left a comment

LPGhatguy May 15, 2018

LPGhatguy May 15, 2018

AmaranthineCodices May 15, 2018

LPGhatguy May 15, 2018

AmaranthineCodices May 15, 2018

LPGhatguy May 16, 2018

AmaranthineCodices May 16, 2018

LPGhatguy May 16, 2018

AmaranthineCodices May 16, 2018

LPGhatguy May 16, 2018

AmaranthineCodices May 16, 2018

LPGhatguy May 16, 2018

LPGhatguy May 16, 2018

createRef API #92

createRef API #92

Conversation

AmaranthineCodices commented May 15, 2018

coveralls commented May 15, 2018 • edited Loading

LPGhatguy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 15, 2018 •

edited

Loading