uniforms icon indicating copy to clipboard operation
uniforms copied to clipboard

ListItemField: Encountered two children with the same key

Open JerryDiemsee opened this issue 4 years ago • 8 comments

Hi,

I'm using the following code to display a ListField with some CustomField:

<ListField name="products" label="Products: " initialCount={1}>
    <ListItemField name="$">
        <ProductSelectFieldUnstyled name="name" label="Product: " />
    </ListItemField>
</ListField>

And my ProductSelectFieldUnstyled returns:

return (
    <SelectField
        name={name}
        label={label}
        allowedValues={productIdsAndTitles}
        transform={(value: any) => value.title}
        disabled={disabled}
    />
);

This works fine, and I got my products displayed, but if I try to select one item, it always selects the first one and in the console, I got the following warning: Warning: Encountered two children with the same key, `[object Object]`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.

I guess I have to set the key in the ProductSelectFieldUnstyled, but I don't know how I can get it. Using useField ? or passing a property from the ListItemField ?

JerryDiemsee avatar Jan 26 '21 10:01 JerryDiemsee

Hi @JerryDiemsee. Based on the duplicate key you've reported, ListField is not part of the problem (it makes dashed keys), but the SelectField itself. The thing is, that there's no support for objects in allowedValues.

EDIT: this would require a lot of work across all themes, therefore I removed the milestone.

radekmie avatar Jan 28 '21 09:01 radekmie

Yeah right, after creating the issue, I started digging and saw the dashed keys in ListField. So I guess the only solution I have is to stick to strings in allowedValues and make sure, on my side, to have unique string, right ?

Can you provide an estimation on the timeframe in which it could be resolved ? Just so I can plan how to deal with it. Thanks !

JerryDiemsee avatar Jan 28 '21 09:01 JerryDiemsee

As a quick fix, wouldn't it be possible to add a keyGetter prop to SelectField, and call it instead of item so that everyone could define its own key, like

const myKeyGetter((item) => item.Id)

From what I understand, this would not require too much work across all themes.

JerryDiemsee avatar Jan 28 '21 10:01 JerryDiemsee

So I guess the only solution I have is to stick to strings in allowedValues and make sure, on my side, to have unique string, right ?

Exactly.

Can you provide an estimation on the timeframe in which it could be resolved ?

Well, not really. However, you are more than welcome to help. And there's no idea (yet) on how to handle it.

As a quick fix, wouldn't it be possible to add a keyGetter prop to SelectField [...]

It would, definitely. But then it becomes a part of the API and, as we are strictly following SemVer, removal of it would be a breaking change. You can create a custom SelectField, that does exactly that. That's why we'd need to think of the whole object handling beforehand.

radekmie avatar Jan 28 '21 12:01 radekmie

Well, not really. However, you are more than welcome to help. And there's no idea (yet) on how to handle it.

Sure. I would be glad to help later. But for now, I'm still new to the web world (2 months), JS, React, ... and I'm still trying to understand what I am doing with all those new libs / deploy process / automations / ... 😅

It would, definitely. But then it becomes a part of the API and, as we are strictly following SemVer, removal of it would be a breaking change. You can create a custom SelectField, that does exactly that. That's why we'd need to think of the whole object handling beforehand.

That makes sense. I got your point.

JerryDiemsee avatar Jan 28 '21 15:01 JerryDiemsee

Hi @JerryDiemsee! It's been a while, but I have found a good and non-hacky workaround. You can add a toString property to your objects and it'll work exactly in the same way as the "key getter" would. I've created a demo CodeSandbox for it. It's causing type errors, but it's something we'll work on (SelectField has to be generic).

EDIT: It's also something we'd need to add to our docs, especially with an example.

radekmie avatar Jul 10 '21 13:07 radekmie

(We've had a team discussion about this issue, and here are the notes.)

We've decided to implement it, but as the proposal is a breaking change, it'll happen in v4.

  • Have a single function called getOptionProps (a better name is needed).
    getOptionProps(value: T): OptionProps
    ​
    type OptionProps = {
        disabled?: boolean; // Replaces `disableItem`.
                            // Defaults to `false`.
        key?: string;       // Introduces a new functionality needed by #862.
                            // Defaults to `value`.
        label?: string;     // Replaces `transform`.
                            // Defaults to `value`.
        value: string;      // Introduces a new functionality.
                            // It has to be unique.
    }
    
    It's entirely backward compatible if you're not using disableItem nor transform.
  • Allow allowedValue to be a list of OptionProps instead of T.
  • Remove disableItem and transform as they are no longer needed.

radekmie avatar Dec 10 '21 10:12 radekmie

As described in #806 and how @radekmie wrote about it, we'll be changing the type of options which will expect you to provide the key explicitly when constructing options array. The allowedValues and transform will be removed.

kestarumper avatar Aug 26 '22 12:08 kestarumper

Hi, this issue has been resolved by #1229. Here's the attached example https://codesandbox.io/s/busy-shirley-osts5t?file=/App.tsx.

I'm closing this issue. Feel free to re-open it if you encounter any problems or if you have other questions.

kestarumper avatar Jun 07 '23 13:06 kestarumper