gcanti / tcomb-form

Forms library for react
https://gcanti.github.io/tcomb-form
MIT License
1.16k stars 136 forks source link

Encountered two children with the same key #152

Closed volkanunsal closed 9 years ago

volkanunsal commented 9 years ago

I have a t.list(t.Str) field that is defined like this:

{
   overtime_holidays_other: t.list(t.Str)
}

It's not customized with a template.

When I add more than two items to it, React throws this warning, and it doesn't accept any new items to the list, nor does it allow me to remove an item from the list:

warning.js:48 Warning: flattenChildren(...): Encountered two children with the same key, `.$=10=10=11=1$=011=20=10=10=105`. Child keys must be unique; when two children share a key, only the first child will be used.
gcanti commented 9 years ago

You seem capable to generate monsters :) This is working for me (I mean I don't see errors when I add rows):

import React from 'react';
import t from 'tcomb-form';

const Form = t.form.Form;

const Type = t.struct({
  overtime_holidays_other: t.list(t.Str)
});

const options = {
};

const App = React.createClass({

  onSubmit(evt) {
    evt.preventDefault();
    var value = this.refs.form.getValue();
    if (value) {
      console.log(value);
    }
  },

  render() {
    return (
      <form onSubmit={this.onSubmit}>
        <Form ref="form"
          type={Type}
          options={options}
        />
        <button className="btn btn-primary">Save</button>
      </form>
    );
  }

});

React.render(<App />, document.getElementById('app'));
volkanunsal commented 9 years ago

Hahaha.. yeah. :) This form is big and ugly. It has a dozen or so fields, and this field only chokes up on this form. I might just use a text field there. I'm not going to worry about it for now.

skrobek commented 9 years ago

@gcanti Hey, I have the same issue with the list of strings only. And what is weird always after add two new items. IMHO should be re-openend.

gcanti commented 9 years ago
skrobek commented 9 years ago

I see that i have v0.5.4 - let me update and check.

gcanti commented 9 years ago

Should be the same for this kind of issue (v0.5.5 adds just a relaxed constraint for the bootstrap skin) https://github.com/gcanti/tcomb-form/releases. However if you can upgrade is even better

skrobek commented 9 years ago

Yes, I have just checked and isse is still there.

Fragments from my code. I'm creating form structure in a loop where list included and excluded are simple inputs lists

getForm:() ->
   for lang, value of ["en", "de", "fr"]
      formItems = {
        name: t.Str
        description: t.Str
        included: t.list(t.Str)
        excluded: t.list(t.Str)
      }
      items[lang] = t.struct(formItems)
    return t.struct(items)

I also have a template for the tab (each language):

  <div className="list-input">
    <div className="list-input">
      {locals.inputs.included}
    </div>
    <div className="list-input">
      {locals.inputs.excluded}
    </div>
 </div>

Which is added to options also in a loop:

for lang, value of @state.locale
      options[lang] = {
        template: @getTabTemplate(lang) 
      ....

And another template for the whole form, where I render tabs for each language:

<div className="form-tabs">
  { for lang, value of @state.locale
    <div key="tab-#{lang}" className="tab tab-btn tab-btn-#{lang}" onClick={_this.changeTab.bind(_this, lang)}>
      {lang}
    </div>
  }
  <div className="clearfix" />
  <br />
  { for lang, value of @state.locale
    <div>{locals.inputs["#{lang}"]}</div>
  }
</div>

No Special options for the excluded and included fields. And at the end form render

<Form type={@getForm()} options={@getFormOptions()} ref="form" value={@state.values} onChange={@onFormChange} />
<hr/>
<div className="form-buttons">
  <button className="btn btn-success" onClick={@submitForm}>
    Save Changes
  </button>
</div>
gcanti commented 9 years ago

@skrobek @evax @volkanunsal

Hi guys, I'm trying hard to break my own code but no luck till now. These are the tests I've done:

My setup:

Attempt 1 (struct of lists):

// struct of lists

const Type = t.struct({
  name: t.Str,
  tags: t.list(t.Str)
});

const App = React.createClass({

  onSubmit(evt) {
    evt.preventDefault();
    var value = this.refs.form.getValue();
    if (value) {
      console.log(value);
    }
  },

  render() {
    return (
      <form onSubmit={this.onSubmit}>
        <Form ref="form"
          type={Type}
        />
        <button className="btn btn-primary">Save</button>
      </form>
    );
  }

});

React.render(<App />, document.getElementById('app'));

Attempt 2 (list of lists):

// list of lists

const Type = t.struct({
  tags: t.list(t.list(t.Str))
});

Attempt 3 (struct of list of structs):

// struct of list of structs

const Type = t.struct({
  tags: t.list(t.struct({
    name: t.Str,
    surname: t.Str
  }))
});

Attempt 4 (custom templates): (@skrobek this is my translation of your gist from coffeescript to ES6. It still works fine, do you spot something I'm missing?)

// test case for issue #152

const locales = ["en", "de", "fr"];

function getForm() {
  const items = {};
  locales.forEach(lang => {
    const formItems = {
      name: t.Str,
      description: t.Str,
      included: t.list(t.Str),
      excluded: t.list(t.Str)
    };
    items[lang] = t.struct(formItems);
  });
  return t.struct(items);
}

function template(locals) {
  return (
    <div className="list-input">
        <div className="list-input">
          {locals.inputs.included}
        </div>
        <div className="list-input">
          {locals.inputs.excluded}
        </div>
     </div>
  );
}

const options = {
  template: function (locals) {
    return (
      <div className="form-tabs">
        <div>{locals.inputs["en"]}</div>
        <div>{locals.inputs["de"]}</div>
        <div>{locals.inputs["fr"]}</div>
      </div>
    );
  },
  fields: {
    en: {
      template: template
    },
    de: {
      template: template
    },
    fr: {
      template: template
    }
  }
};

const App = React.createClass({

  onSubmit(evt) {
    evt.preventDefault();
    var value = this.refs.form.getValue();
    if (value) {
      console.log(value);
    }
  },

  render() {
    return (
      <form onSubmit={this.onSubmit}>
        <Form ref="form"
          type={getForm()}
          options={options}
        />
        <button className="btn btn-primary">Save</button>
      </form>
    );
  }

});

React.render(<App />, document.getElementById('app'));

I'm sorry, I know it's not helpful but I just can't find a way to reproduce this issue.

evax commented 9 years ago

Thanks for your efforts!

I'm still trying to pin point my failing case, and here's what I have so far:

var React = require('react');
var t = require('tcomb-form');

var Child = t.struct({
  id: t.maybe(t.Num),
  num: t.Str,
  str: t.Str
});

var Parent = t.struct({
  id: t.maybe(t.Num),
  str: t.Str,
  children: t.list(Child),
  num: t.Num
}, 'Parent');

var Form = t.form.Form;

var template = function(locals) {
  var inputs = locals.inputs;
  return (
    <fieldset>
      {inputs.id}
      <div>
        {inputs.str}
      </div>
      <div>
        {inputs.children}
      </div>
      <div>
        {inputs.num}
      </div>
    </fieldset>
  );
};

var options = {
  template: template,
  fields: {
    id: {type: 'hidden'},
    str: {label: 'String Value'},
    num: {label: 'Number Value'},
    children: {
      item: {
        fields: {
          //id: {type: 'hidden'}
        }
      }
    }
  }
};

var App = React.createClass({
  render: function() {
    return (
      <Form type={Parent} options={options} />
    );
  }
});

React.render(<App />, document.getElementById('app'));

This small example is what I could derive from my code but it's working.

In my code though if options.fields.children.item.fields is missing or empty I get the same key issue.

As long as options.fields.children.item.fields contains at least one entry corresponding to an existing Child property, even if empty itself (e.g. id: {}) then it works.

This is a bit more precise than what I last mentioned in #157, hope this helps.

skrobek commented 9 years ago

@gcanti Hey, thanks for testing. There is one difference in my code. In the get template function for the tab, I need a language inside so the function looks a little bit different

getTabTemplate: (lang) ->
    return (locals) =>
      <div className="tab-content hide tab-#{lang}"> # here i'm using lang variable
        <div className="row">
          <div className="col-xs-8">{locals.inputs.name}</div>
          <div className="col-xs-4">{locals.inputs.price}</div>
        </div>
        <hr/>
        <div>{locals.inputs.description}</div>
        <div className="list-input">
          {locals.inputs.included}
          <div className="text-muted">
             Please add featuers which are included in insurance.
          </div>
        </div>
        <div className="list-input">
          {locals.inputs.excluded}
          <div className="text-muted">
            Please add featuers which are excluded from insurance.
          </div>
        </div>
      </div>

And then inside options I have:

for lang, value of @state.locale
      options[lang] = {
        template: @getTabTemplate(lang)
skrobek commented 9 years ago

@gcanti I still can't find an issue, so maybe better will be if I will post full file: https://gist.github.com/skrobek/bd48064bc207b939dc2c

jonaswindey commented 9 years ago

I have the same issue with 0.6, a very basic test form to reproduce:

Model:

import t from 'tcomb-form';

const InquiryModel = t.struct({
  ref: t.Str
});

export default t.list(InquiryModel);

Form:

  _onChange(value) {
    this.setState({value});
  }

<Form type={InquiryModel}
                  ref="form"
                  value={this.state.value}
                  onChange={this._onChange.bind(this)}
              />

The error occurs when I click "add" to create a new inquiry. When I remove the onChange handler, the error goes away.

gcanti commented 9 years ago

Awesome @jonaswindey! Now I have a test case. I'll work on it asap. Thanks a lot!

gcanti commented 9 years ago

@jonaswindey

I think I've found a fix. Just published

can you test it out?

cc @skrobek @evax @volkanunsal

skrobek commented 9 years ago

@gcanti Tested with v0.5.6. Works like a charm now. Thanks ! :-)

evax commented 9 years ago

Tested with v0.6.1 and works perfectly, even with my corner case, thank you @gcanti!

gcanti commented 9 years ago

Great. Thanks to all for your help