React component children passed as `children` props instead of additional args to React.createElement()

Here is some sample rescript that appears to pass children: null to a React.createElement() call in the generated javascript when in fact there are children present in the jsx. I’m debugging some issues I’m seeing with my Headless UI bindings and was wondering if this might have any implications during rendering.

Really it has this problem…
I found decision with fragment:

@react.component
let make = () =>
  <Headless.Menu.Items static=true>
    <>
      <Headless.Menu.Item>
        {_ => <p> {React.string("Item")} </p>}
      </Headless.Menu.Item>
      <Headless.Menu.Item>
        {_ => <p> {React.string("Item 2")} </p>}
      </Headless.Menu.Item>
      <Headless.Menu.Item>
        {_ => <p> {React.string("Item 3")} </p>}
      </Headless.Menu.Item>
	</>
  </Headless.Menu.Items>

But I think that it’s problem.
I passed the code in TS Playground and there is not this problem.

That still appears to be problematic, for a similar reason. Here is the generated js:

function Playground(Props) {
  return React.createElement(React$1.Menu.Items, {
              static: true,
              children: React.createElement(React.Fragment...

It’s passing children as the children prop instead of the third parameter to createElement, which also triggers the following eslint warning:

Do not pass children as props. Instead, pass them as additional arguments to React.createElement.eslint react/no-children-prop

Yes…You are right…
Children should not be in the props object.

Hi @pheuter

My understanding is that passing children as a prop to React.createElement() is valid.

When JSX children are present, they will override the children prop.

That eslint warning is probably to minimise confusion over which children will be used (i.e. prop children or JSX children).

Unless there is a technical reason for the current implementation in the compiler, it might be a nice change to always have children passed as the 3rd argument to createElement. The team might be able to explain further.

1 Like

The new jsx transform will require passing children as props so I wouldn’t think there’s much reason to change to appease a lint rule for the current createElement implementation?

2 Likes

Looks like the var-args are populated over the explicit children prop if the children are passed in that way

2 Likes

Right on, good call on checking the source of ReactElement. Looks like props.children is the source of truth for children, and will be overwritten if 3 or more args are passed to createElement.

1 Like

How do I fix this? I’m getting this very error when trying to build a react app with nextjs.

ReScript:

@react.component
let default = () => {
  <Page> {"123"->React.string} </Page>
}

Generated JS:

function Demo$default(Props) {
  return React.createElement(Page.make, {
              children: "123"
            });
}

Error:

./src/pages/demo.bs.js
8:10  Error: Do not pass children as props. Instead, pass them as additional arguments to React.createElement.  react/no-children-prop

This looks like an issue on the Next.js side. ReScriptReact is creating a standard React component here.

It rather seems to be eslint:

ESLint is mostly designed for JavaScript or TypeScript project. Most of its rules don’t make sense for ReScript projects.

1 Like

You might find this thread interesting.