React incorrectly assigning `key` property to non-elements passed as children

[littensy]

When creating an element and the following conditions are met, React forcefully assigns a key prop to tables passed as children, which can trigger a misleading createTextInstance error if the table is not an actual React element:

1. false or nil is passed as a child (usually through conditional rendering)
2. The falsy value is followed by a table
3. The component re-renders and returns this element

The following patterns throw an error in Roblox, but not in ReactJS:

-- 🔴 Errors in Roblox, but works as expected in ReactJS v17
createElement("Frame", {}, false, children)
createElement("Frame", {}, { false, children })
createElement("Frame", { children = { false, children } })

-- 🟢 OK in both
createElement("Frame", {}, children, false)
createElement("Frame", {}, { false }, children)

This is likely a bug, as the error originates from a Roblox deviation (see Source of error).

Original issue: https://github.com/jsdotlua/react-lua/issues/42

Cause

Consider the following component:

local function App()
    local state, setState = React.useState(1)

    React.useEffect(function()
        setState(2)
    end, {})

    local children = {}

    task.defer(function()
        print(children) -- Re-rendering sets children.key = 2
    end)

    return React.createElement("Frame", {}, false, children)
end

Rendering this component throws an error related to createTextInstance, but the source of the error is revealed when printing children, resulting in the following output:

▼  {
    ["key"] = 2
}

React is assigning a stable key to the table of children. This likely errors because React interprets the table as a children table with number 2 being a child with a stable key of "key", which then throws a createTextInstance error when trying to render the number 2.

Source of error

The misplaced key property originates from a Roblox deviation in ReactChildFiber.new:780:.

Adjusting the code to check for newChild["$$typeof"] before assigning a stable key prevents assigning keys to non-elements. This can be checked either on the previously mentioned line or in the assignStableKey() function.

local existingChildrenKey
-- ROBLOX performance: avoid repeated indexing to $$typeof
local newChildTypeof = newChild["$$typeof"]
-- ROBLOX deviation: Roact stable keys - forward children table key to
-- child if applicable
if newChildTypeof then
    assignStableKey(tableKey, newChild)
end

local function assignStableKey(tableKey: any?, newChild: Object): ()
    -- ...
    if newChild.key == nil and newChild["$$typeof"] then

It's also worth noting that assignStableKey() is called many times in this module, and I don't know if those other calls have similar issues, or if these proposed changes have problematic side effects.

Repro

Here's a minimal repro of this error using an up-to-date build of React as of commit 3aeb76282fa58ab26e97472fb0c07db9d4372689:

local React = require(workspace.RoactAlignment.React)
local ReactRoblox = require(workspace.RoactAlignment.ReactRoblox)

local function App()
	local state, setState = React.useState(1)

	-- The error occurs after re-rendering
	React.useEffect(function()
		setState(2)
	end, {})

	local children = {}

	task.defer(function()
		print(children) --> {} on mount, { key = 2 } during re-render
	end)

	return React.createElement("Frame", {}, false, children)
end

local root = ReactRoblox.createRoot(Instance.new("Folder"))

root:render(React.createElement(App))

Expected output

{}
{}

Actual output

  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  UNIMPLEMENTED ERROR: createTextInstance
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  UNIMPLEMENTED ERROR: createTextInstance
  Error: FIXME (roblox): createTextInstance is unimplemented
  Workspace.RoactAlignment.Scheduler.forks.SchedulerHostConfig.default:315: Workspace.RoactAlignment.Scheduler.forks.SchedulerHostConfig.default:292: 
------ Error caught by React ------
FIXME (roblox): createTextInstance is unimplemented
------ Error caught by React ------
Workspace.RoactAlignment.ReactRoblox.client.ReactRobloxHostConfig:17 function unimplemented
Workspace.RoactAlignment.ReactRoblox.client.ReactRobloxHostConfig:460
Workspace.RoactAlignment.ReactReconciler.ReactFiberCompleteWork.new:1024 function completeWork
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:263
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:2014
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:1980
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:1858
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:1807
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:940
Workspace.RoactAlignment.ReactReconciler.ReactFiberWorkLoop.new:856
Workspace.RoactAlignment.Scheduler.Scheduler:306
Workspace.RoactAlignment.Scheduler.Scheduler:262
Workspace.RoactAlignment.Scheduler.forks.SchedulerHostConfig.default:240 function doWork
Workspace.RoactAlignment.Scheduler.forks.SchedulerHostConfig.default:276 function performWorkUntilDeadline

  Stack Begin
  Script 'Workspace.RoactAlignment.Scheduler.forks.SchedulerHostConfig.default', Line 315
  Stack End

{}
{ ["key"] = 2 }
{ ["key"] = 2 }