listChildren array elements have no key

[mAAdhaTTah]

Bug description

The Task component convert its non-array children into an array, but in doing so, doesn't provide a key to the element within, producing the following error:

Warning: Each child in a list should have a unique "key" prop.

Check the render method of `Task`. It was passed a child from EnvSetup. See https://reactjs.org/link/warning-keys for more information.
    at Text (/[redacted]/cli/node_modules/ink/build/components/Text.js:12:17)
    in Task (created by EnvSetup)
    in ink-box (created by Box)
    in Box (created by TaskList)
    in TaskList (created by EnvSetup)
    in EnvSetup
    in InternalApp

Reproduction steps

1. Pass Task a single element child + isExpanded.
2. Run ink.
3. See the above error.

Environment

• ink-task-list version: 1.0.0
• Webpack version: Pastel
• Operating System: OSX
• Node version: 15.14.0
• Package manager (npm/yarn/pnpm) and version: yarn 1.22.10

[privatenumber]

Works fine for me:

import React from 'react';
import { render } from 'ink';
import { Task } from 'ink-task-list';

render(
	<Task
		label="Label 1"
		isExpanded
	>
		<Task
			label="Label 2"
			state="loading"
		/>
	</Task>
);

Can you provide reproduction code?

[mAAdhaTTah]

Yeah, so I was creating children in a separate function (I'm using the expanded area to prompt questions about currently running tasks) and passed it as a prop, which caused it:

<Task label="Label 1" isExpanded children={<Text>Label 2</Text>} />

(I used Text for simplicity but did repro initially with Task as well.)

It's a bit odd because both cases, the children comes into Task as an object, rather than an array, but I did notice this part of the message:

Check the render method of Task. It was passed a child from Hello.

(emphasis mine)

which suggests (to me) that React is treating children passed via JSX differently from children passed via prop.

This behavior also appears consistent between React 16 + 17.

FWIW, if no children are provided and you render the expanded box anyway, does it take up space? Alternatively, might be better to use the React.Children methods for this. I can open a PR for this.

[mAAdhaTTah]

Using React.Children.toArray instead of the current Array.isArray ternary resolved the issue for me locally.

[privatenumber]

Thanks for the details and fix.

Out of curiosity, what's the reasoning behind passing in children as a prop?

[github-actions]

🎉 This issue has been resolved in version 1.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mAAdhaTTah]

@privatenumber So I've got an array that's a list of tasks:

[{
  label: 'A task',
  run: async () => { ... do stuff ... }
}]

Those tasks will throw errors if they need a prompt (e.g. FileExistsError means we're attempting to create a file that already exists and need to prompt the user for permission to overwrite). We run the tasks in a for loop and wrap the run in a try / catch, then in the catch, set that we're prompted for something.

In render, we map over that task list:

tasks.map((task, i) => (
  <TaskList>
    <Task label={task.label) {...getTaskProps(task, i, state)} />
  </TaskList>

getTaskProps looks at state and its properties + i and will return something like:

return { status: 'loading' / 'success' / etc, isExpanded: true / false, children:  null / <ConfirmInput ... /> }

That's a long-winded way of saying "children is dynamic". While right now it's just "can I overwrite this file?", this pattern is already expanding to other questions e.g. "give me sudo plz", so rendering children the "normal JSX way" wasn't really feasible.

Other than this minor issue, the lib has worked great for this use case, btw. Great work, thank you!

[privatenumber]

Ahh I see. Thanks for the explanation!