-
Notifications
You must be signed in to change notification settings - Fork 143
createRef API #92
createRef API #92
Changes from 11 commits
4e773ef
82b2330
72b907c
35a9df7
d002373
e2f452f
b624801
9ce1d84
3cda6f7
1f86eb1
878ecc5
598af40
6f770ab
8c6cb36
4614af4
3378d4a
336db31
9de73c7
b139cc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,20 @@ local function isPortal(element) | |
return element.component == Core.Portal | ||
end | ||
|
||
--[[ | ||
Sets the value of a reference to a new rendered object. | ||
Correctly handles both function-style and object-style refs. | ||
]] | ||
local function applyRef(ref, newRbx) | ||
if not ref then return end | ||
|
||
if type(ref) == "table" then | ||
ref.current = newRbx | ||
else | ||
ref(newRbx) | ||
end | ||
end | ||
|
||
local Reconciler = {} | ||
|
||
Reconciler._singleEventManager = SingleEventManager.new() | ||
|
@@ -93,9 +107,7 @@ function Reconciler.unmount(instanceHandle) | |
|
||
-- Kill refs before we make changes, since any mutations past this point | ||
-- aren't relevant to components. | ||
if element.props[Core.Ref] then | ||
element.props[Core.Ref](nil) | ||
end | ||
applyRef(element.props[Core.Ref], nil) | ||
|
||
for _, child in pairs(instanceHandle._children) do | ||
Reconciler.unmount(child) | ||
|
@@ -173,9 +185,7 @@ function Reconciler._mountInternal(element, parent, key, context) | |
rbx.Parent = parent | ||
|
||
-- Attach ref values, since the instance is initialized now. | ||
if element.props[Core.Ref] then | ||
element.props[Core.Ref](rbx) | ||
end | ||
applyRef(element.props[Core.Ref], rbx) | ||
|
||
return { | ||
[isInstanceHandle] = true, | ||
|
@@ -323,7 +333,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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to update the branch on line 349 that applies |
||
end | ||
|
||
-- Update properties and children of the Roblox object. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,39 @@ | ||
return function() | ||
local Core = require(script.Parent.Core) | ||
local Reconciler = require(script.Parent.Reconciler) | ||
local createRef = require(script.Parent.createRef) | ||
|
||
it("should mount booleans as nil", function() | ||
local booleanReified = Reconciler.mount(false) | ||
expect(booleanReified).to.never.be.ok() | ||
end) | ||
|
||
it("should handle object references properly", function() | ||
local objectRef = createRef() | ||
local element = Core.createElement("StringValue", { | ||
[Core.Ref] = objectRef, | ||
}) | ||
|
||
local handle = Reconciler.mount(element) | ||
expect(objectRef.current).to.be.ok() | ||
Reconciler.unmount(handle) | ||
expect(objectRef.current).to.never.be.ok() | ||
end) | ||
|
||
it("should handle function references properly", function() | ||
local currentRbx | ||
|
||
local function ref(rbx) | ||
currentRbx = rbx | ||
end | ||
|
||
local element = Core.createElement("StringValue", { | ||
[Core.Ref] = ref, | ||
}) | ||
|
||
local handle = Reconciler.mount(element) | ||
expect(currentRbx).to.be.ok() | ||
Reconciler.unmount(handle) | ||
expect(currentRbx).to.never.be.ok() | ||
end) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test that makes sure that refs can be unattached and attached through |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
--[[ | ||
Provides an API for acquiring a reference to a reified object. This | ||
API is designed to mimic React 16.3's createRef API. | ||
|
||
See: | ||
* https://reactjs.org/docs/refs-and-the-dom.html | ||
* https://reactjs.org/blog/2018/03/29/react-v-16-3.html#createref-api | ||
]] | ||
|
||
local refMetatable = { | ||
__tostring = function(self) | ||
return ("RoactReference(%q)"):format(tostring(self.current)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it now, do you think we should just format these with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's true - I'm used to quoting everything, but in this case it doesn't do much good! Fixed now. |
||
end, | ||
} | ||
|
||
return function() | ||
return setmetatable({ | ||
current = nil, | ||
}, refMetatable) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
return function() | ||
local createRef = require(script.Parent.createRef) | ||
|
||
it("should create refs", function() | ||
expect(createRef()).to.be.ok() | ||
end) | ||
|
||
it("should support tostring on refs", function() | ||
local ref = createRef() | ||
expect(tostring(ref)).to.equal("RoactReference(\"nil\")") | ||
end) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're going to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fair, done! |
||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's explicitly check
if ref == nil
and then put the return statement on its own line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now!