React Lua incorrectly assigning `key` prop to `children` array, causing misleading `createTextInstance` errors

[littensy]

When declaring an Instance with createElement and the following conditions are met, React Lua assigns a key prop to a table of children, which triggers a createTextInstance error:

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

Generally, these patterns throw the error when condition is false or nil (see Deviation from React for why this can be a problem):

-- 🔴 Errors in React Lua, but works as intended in React JS v17
e("Frame", {}, condition and child, children)
e("Frame", {}, { condition and child, children })
e("Frame", { children = { condition and child, children } })

-- 🟢 OK
e("Frame", {}, children, condition and child)
e("Frame", {}, { condition and child }, children)

Explanation

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 Lua is incorrectly assigning a stable key to the table of children. This errors because it gets interpreted as the number 2 being a child with a stable key of "key", which then throws a createTextInstance error when trying to render the number 2.

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

Potential solution

Adjusting this 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. It does not have a significant impact on my existing React projects, but I'm not confident in this solution.

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

Deviation from React

I'm mainly reporting this as an issue because throwing a text error here is a deviation from React and it's problematic for JSX compilers emitting code for React Lua.

JSX compilers like Babel and the one in roblox-ts output this pattern when a conditional element is followed by an array of child elements, and JSX compilers for React Lua may also rely on this behavior:

<div>
  {condition && <div />}
  {children}
</div>

// Babel ("Classic" runtime)
React.createElement("div", null, condition && React.createElement("div", null), children);

// Babel ("Automatic" runtime)
_jsxs("div", {
  children: [condition && _jsx("div", {}), children]
});

-- roblox-ts, using <frame>
React.createElement("frame", nil, condition and React.createElement("frame"), children)

Outside of JSX compilation, this error may come up when attempting to pass two tables of children to createElement, where the first table is rendered conditionally.

Repro

Here's a minimal repro of this error using the latest production build of React Lua:

local React = require(workspace.ReactLua.React)
local ReactRoblox = require(workspace.ReactLua.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))

Error Logs

The repro throws this error, using the react-lua.rbxm file from release 17.2.1:

  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  UNIMPLEMENTED ERROR: createTextInstance
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  UNIMPLEMENTED ERROR: createTextInstance
  Error: FIXME (roblox): createTextInstance is unimplemented
  [string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.forks.SchedulerHostConfig.default"]:139: [string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.forks.SchedulerHostConfig.default"]:116: 
------ Error caught by React ------
FIXME (roblox): createTextInstance is unimplemented
------ Error caught by React ------
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-roblox.client.ReactRobloxHostConfig"]:17 function unimplemented
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-roblox.client.ReactRobloxHostConfig"]:462
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberCompleteWork.new"]:1020 function completeWork
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:254
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:2001
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:1967
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:1845
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:1794
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:930
[string "Workspace.ReactLua.node_modules.@jsdotlua.react-reconciler.ReactFiberWorkLoop.new"]:846
[string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.Scheduler"]:303
[string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.Scheduler"]:259
[string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.forks.SchedulerHostConfig.default"]:80 function doWork
[string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.forks.SchedulerHostConfig.default"]:103 function performWorkUntilDeadline

  Stack Begin
  Script '[string "Workspace.ReactLua.node_modules.@jsdotlua.scheduler.forks.SchedulerHostConfig.default"]', Line 139
  Stack End