ProductiveRage / Bridge.React

Bindings for Bridge.NET for React - write React applications in C#!
MIT License
74 stars 14 forks source link

Key not being set when creating new child components #45

Closed dnaustin closed 6 years ago

dnaustin commented 6 years ago

I have a table that is showing the results returned from an api call. As the number of results returned each time will vary, I have to dynamically create the rows and therefore give each row a key. I have encapsulated the row into it's own child component so that when rendering the table the following occurs:

public override ReactElement Render()
{ 
    var resultRows = this.props.Results.Select(result => new ResultRow(result));
    return DOM.Table(null,
        DOM.THead(null,
            DOM.TR(null,
                DOM.TH("Example table")
            )
        ),
        DOM.TBody(null, resultRows))
    );
}

Inside of the ResultRow element, I set the Key as part of the render:

public override ReactElement Render()
{ 
    return DOM.TR(new TableCellAttributes { Key = (int)this.props.Result.Key },
        DOM.TD(null, this.props.Result.Value)
    );
}

When I fire up the app and create rows, the "Each child in an array or iterator should have a unique "key" prop" warning appears. However, if I incorporate the row into the main table component (i.e. the Select transforms straight to DOM.TR), it sets the key as expected.

ProductiveRage commented 6 years ago

I think that the issue is that if you have a custom component then React wants to be able to find a unique key on the props for that component (if you're using that component as part of a dynamic list for child components).

In your code, you're setting a "Key" on the table row returning from Render but there isn't a "Key" property on the props class for the ResultRow component itself.

I've included an example below to try to illustrate. Note that I'm not setting a Key value when I call DOM.TR because there is a Key property on the Props class of ResultRow -

using System.Linq;
using Bridge.Html5;
using ProductiveRage.Immutable;

namespace Bridge.React.Examples
{
    public sealed class App
    {
        public static void Main()
        {
            var resultRows = new[] { "One", "Two", "Three" }.Select((text, index) => new ResultRow(index, text));
            React.Render(
                DOM.Table(null,
                    DOM.THead(
                        DOM.TR(
                            DOM.TH("Example table")
                        )
                    ),
                    DOM.TBody(resultRows)
                ),
                Document.GetElementById("main")
            );
        }
    }
    public sealed class ResultRow : PureComponent<ResultRow.Props>
    {
        public ResultRow(int key, string text) : base(new Props(key, text)) { }

        public override ReactElement Render()
        {
            return DOM.TR(
                DOM.TD(props.Text)
            );
        }

        public sealed class Props : IAmImmutable
        {
            public Props(int key, string text)
            {
                this.CtorSet(_ => _.Key, key);
                this.CtorSet(_ => _.Text, text);
            }
            public int Key { get; }
            public string Text { get; }
        }
    }
}

You may be interested to note that there are some component factory method overloads for cases where you don't want to set any attributes, so I'm able to say this:

DOM.TBody(resultRows)

instead of this:

DOM.TBody(null, resultRows)

The same applies to DOM.THead and DOM.TD - it generally works in the case where you have a single argument other attributes, so it will work if you're setting a single child component that is a string or a single child component that is a ReactElement or that is a custom component and it will work if you give it an enumerable of any of these (as you can see in the DOM.TBody call).

It won't work if you need to specify multiple child component arguments, as with the DOM.Table call - but it's still a nice little feature that can make your code tidier!

A final note: since the "Key" property only needs to be available to React when you're creating custom components, you won't need to access it in many cases (in the example code above, the ResultRow code doesn't have to access its "Key" property - the fact that it is on the props class is enough for React). To make this common case easier, the Bridge.React library has a PropsWithKey base class that could be used to refactor the Props class above from this:

public sealed class Props : IAmImmutable
{
    public Props(int key, string text)
    {
        this.CtorSet(_ => _.Key, key);
        this.CtorSet(_ => _.Text, text);
    }
    public int Key { get; }
    public string Text { get; }
}

to this:

public sealed class Props : PropsWithKey, IAmImmutable
{
    public Props(int key, string text) : base(key)
    {
        this.CtorSet(_ => _.Text, text);
    }
    public string Text { get; }
}

It's not essential, it's just a nice convention and means that you have slightly less code to deal with!

dnaustin commented 6 years ago

You are absolutely correct with your analysis! Changing to PropsWithKey and passing the key in works as expected. Thanks for your help again.