jsdotlua / jsdotlua/react-lua

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

Ouverte
#42 0 commentaires 20 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Langage dominant
Luau
Étoiles
569
Forks
25
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

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

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez dans modules/react-reconciler/src/ReactChildFiber.new.luau autour de la ligne 779 et examinez assignStableKey() près de la ligne 581, en vous concentrant sur la vérification $$typeof mentionnée. Exécutez le repro minimal de React.createElement avec un falsy child suivi de children et vérifiez qu’un nouveau rendu n’ajoute plus de key à la table children et ne produit pas d’erreur createTextInstance.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
lua
Domaine
frontend
Type d'issue
Bug
Difficulté
3/5
Temps estimé
1-2 jours
Activité
À l'abandon
Clarté
Clairement spécifiée
Accessibilité débutants
52/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.