jsdotlua / jsdotlua/react-lua

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

オープン
#42 コメント 0 件 リアクション 20 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
Luau
スター
569
フォーク
25
PR マージ指標
30日以内にマージされた PR はありません

説明

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

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

modules/react-reconciler/src/ReactChildFiber.new.luau の 779 行目付近から始め、581 行目付近の assignStableKey() を調べ、言及されている $$typeof チェックに注目してください。falsy child の後に children が続く最小限の React.createElement repro を実行し、再レンダリングによって children table に key が追加されなくなり、createTextInstance エラーも発生しないことを確認してください。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
lua
領域
frontend
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
停滞
明瞭さ
明確に書かれている
初心者へのやさしさ
52/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。