roact icon indicating copy to clipboard operation
roact copied to clipboard

Newly assigned ref is being ignored

Open MagiMaster opened this issue 6 years ago • 2 comments

In some situations when replacing a ref, the new ref does not receive a pointer to the instance during render. After some digging, it seems like the old ref is still getting used during the updateHostNode call.

MagiMaster avatar Aug 08 '19 22:08 MagiMaster

It's been a little bit so I might've lost some context, but we dug into this issue and I think we know what's happening.

When transitioning a ref from one component to another, there's potentially a period of time when a ref is being used by two component simultaneously, and the unmount of one component steps on the mount of another component.

Consider this code example:

local ref = Roact.createRef()

local state1 = Roact.createFragment({
	A = Roact.createElement("Folder", {
		[Roact.Ref] = ref,
	}),
	B = Roact.createElement("Folder", {
		[Roact.Ref] = nil,
	}),
})

local tree = Roact.mount(state1)

local state2 = Roact.createFragment({
	A = Roact.createElement("Folder", {
		[Roact.Ref] = nil,
	}),
	B = Roact.createElement("Folder", {
		[Roact.Ref] = ref,
	}),
})

Roact.update(tree, state2)

One possible sequence of events for Roact currently is:

  1. A is mounted and ref is attached (ref.current = A)
  2. B is mounted (ref.current = A)
  3. B is updated and ref is attached (ref.current = B)
  4. A is updated and ref is detached (ref.current = nil)

This will not always occur because traversal order of dictionaries in Lua is undefined. This error is probably incredibly difficult to debug because of this.

The fix is to change our unmounting logic in step 4 to only detach the ref if it's still attached to A and otherwise leave it alone.

LPGhatguy avatar Sep 10 '19 18:09 LPGhatguy

Also running into this bug. What I was trying to have a ref point to the selected object in a list.

[Roact.Ref] = self.state.selectedPlayer == player and self.selectedRef or nil,

The issue seems to go deeper than just the ref being detached as in didUpdate the ref seemed to point to the last selected object and not nil.

This seemed to be further proved by the solution to this, I changed this to use a function as a ref like so:

local function refUpdatedFunction(rbx)
	if player == self.state.selectedPlayer then --Doesn't fix the issue without this check.
		self.selectedPlayerRef = rbx
	end
end

[Roact.Ref] = self.state.selectedPlayer == player and refUpdatedFunction or nil,

Haven't really looked into this issue much after I found the workaround but happy to help make a reproduction if necessary.

ConorGriffin37 avatar Sep 13 '19 10:09 ConorGriffin37