Trouble with hidden elements updating in subforms

[awhillas]

Version

• tcomb-form v0.8.2
• react v0.14.8

  Expected behaviour

Should updated the subform when the Redux store values are programatically updated i.e. not via onChange interactions.

Actual behaviour

Instead it only updates the subform value="" if another visible element on the subform is updated. Otherwise it seems to remain unchanged. This causes a validation error as the field is required and the form to not submit with no error message.

Steps to reproduce

1. Update required element value of subform in Redux store which is passed to the form as this.props.form.values
2. Submit form. Validation fails silently.
3. Change a visible element of the subform.
4. Submit form. Success?!

   Stack trace and console log

I hacked the tcomb-forms Component class and put log messages in to see what is being called when. For some reason as soon as the hidden element is updated the subform values immediately get the value and no re-render is triggered?

Here is my code first the tcomb stuff:

// The subform struct...
const Variation = t.struct({
  name: t.maybe(t.String),
  file: Positive, // same as from the tcomb-form docs.
  preview: Positive, // file id. Set after dropzone finished uploading and we get an ID back from the server.
  // Variation specific stuff
  tempo: Positive,   // an optional number
  // ..
});

Now the form component

class TrackCreateForm extends React.Component {
  // Form configuration
  static options = {
      // ...
    },
    fields: {
      variation_set: {  // sub form
        fields: {
          preview: { type: 'hidden' },   // This is the hidden field we're updating after the upload to server finishes
           // ...
        }
      },
    }
  }
  // These are for testing. Remove when going live.
  static initValues = { ... }

  componentWillMount() {
    // Initial values for testing...
    let { type, tagsOptions } = this.getForm();
    this.props.onNewForm( type, tagsOptions, ... );  // create form data in Redux store
  }

  componentWillReceiveProps(nextProps) {
    if (this.props.previewId == undefined && nextProps.previewId) {
      // preview file finished uploading.
      this.updateForm({ variation_set: { preview: nextProps.previewId } });  // this is where redux is updated
    }
  }

  onSubmit(evt) {
    evt.preventDefault();
    let values = this.refs.form.getValue();
    if (values) {
      // ... etc
    }
  }

  getForm() {
    let type = t.struct({
      variation_set: Variation, // the subform with problems
    });
    return {type}
  }

  updateForm(newValues = {}) {
    let { type } = this.getForm();
    this.props.onUpdateFormOptions(  // update the redux form
      this.props.formId,
      type,
      newValues,
      ...
    );
  }

  onChange(value, path) {
    // Make this a controlled component
    this.refs.form.getComponent(path).validate()
    this.updateForm(value);
    this.forceUpdate(); // fool the default shouldComponentUpdate. Does SFA.
  }

  render() {
    let { form, previewId, formId } = this.props;
    console.log(form);  // debug
    if(form && form.type) {
      console.log("Rendering form, \w previewId:", form.values.variation_set.preview);  // check we're getting the values, and we are.
      let button = (form.values.variation_set.preview)
        ? <Button type="submit" bsStyle="primary" bsSize="large" active >Save!</Button>
        : <Button type="submit" bsStyle="primary" bsSize="large" disabled >Upload a preview file </Button>
      ;
      return (
        <div className="form-horizontal">
          <form onSubmit={this.onSubmit.bind(this)} id={formId}>
            <t.form.Form ref="form" 
              type={form.type}
              options={form.options}
              value={form.values}  // striaght out-a-redux
              onChange={this.onChange.bind(this)}  // Make this a controlled component
            />
            <div>* = required field</div>
            {button}
          </form>
        </div>
      )
    }
    else {
      return <div>loading...</div>;
    }
  }
}

[gcanti]

Hi @awhillas,

That's a lot of code, could you please provide a minimal example reproducing the issue?

[awhillas]

Sorry, have cleaned it up a bit. Only the getForm and updateForm are non-standard React stuff and they are fairly straight forward.

I can't see how the values for subforms, Structs, are checked. In your code, as far as i can tell, you are just comparing one deeply nested object to another directly with !== in Component.prototype.shouldComponentUpdate? Somehow, when every i update the redux store your subcomponents seem to already have the new values and thus it doesn't trigger a re-render?

[gcanti]

In your code, as far as i can tell, you are just comparing one deeply nested object to another directly with !== in Component.prototype.shouldComponentUpdate?

Correct.

Somehow, when every i update the redux store your subcomponents seem to already have the new values and thus it doesn't trigger a re-render?

Yes, every component keeps a copy of the new value in its state, if the value passed in to the value prop is not changed (=== check) then it won't re-render. Make sure that if you override the value also the reference is changed.

[awhillas]

Your aware that comparing deeply nested objects in Javascript fails if they are not the same object in memory, right? From the console.logs i'm doing it doesn't appear that the sub-Structs are recursively checking each of their sub components when they get a change all the time? At least this is the behaviour I'm seeing. I can't see you've tested this ether? Their seems to also be a difference in what the local this.state has and what is actually rendered to HTML, the problem being that the HTML is used for validation not the store value. Am i reading this right?

[gcanti]

Your aware that comparing deeply nested objects in Javascript fails if they are not the same object in memory, right?

Yes, that's the reason why I say that if you pass a value that doesn't originate from onChange interactions and you want to force a re-rendering you have to make sure that the references are changed (as it happens using immutable structures). Since you are using redux I assume that is what's going on in your code. Or you are using mutable structures?

[awhillas]

Yeah, my reducers always return new versions of the store if there is a change. I'm not using Immutable.js but I'll have a look at it. here's my update reducer (i'm trying to create a generic forms handing store):

export const aFormReducer = (state, action) => {
  // console.log(state, action);
  if (action.meta.id !== state.id)
    return state  // wrong form

  switch (action.type) {
   // ...
    case acts.UPDATE:
      let { form, options, values } = action.payload;
      return {
        id: state.id,
        form: (form) ? form: state.form,
        options: mergeDeep(Object.assign({}, state.options), options),
        values: mergeDeep(Object.assign({}, state.values), values)
      };

    default:
      return state
  }
}

export const forms = (state = initialState, action) => {
  switch (action.type) {

    case acts.NEW:
      // Append new form to list. Payload should just be the tcomb-form options.
      return [...state, action.payload];

    case acts.POST_REQUEST:
    case acts.POST_FAILURE:
    case acts.UPDATE:
      return state.map((aForm) => aFormReducer(aForm, action));

    case acts.POST_SUCCESS:
      // kill the form since we don't need it anymore.
      return state.filter(form => form.id !== action.meta.id)

    default:
      return state;
  }
}

export default forms

basically action.payload always has the new state and this is my mergeDeep code (which i nick'd from somewhere on stackoverflow):

/**
 * Simple is object check.
 * @param item
 * @returns {boolean}
 */
export function isObject(item) {
  return (item && typeof item === 'object' && !Array.isArray(item) && item !== null);
}

/**
 * Deep merge two objects.
 * @param target
 * @param source
 */
export default function mergeDeep(target, source) {
  if (isObject(target) && isObject(source)) {
    Object.keys(source).forEach(key => {
      if (isObject(source[key])) {
        if (!target[key]) Object.assign(target, { [key]: {} });
        mergeDeep(target[key], source[key]);
      } else {
        Object.assign(target, { [key]: source[key] });
      }
    });
  }
  return target;
}

but i always pass it a clone of the data via Object.assign({}, ...)

[gcanti]

The problem seems your mergeDeep function:

var sub = {
  b: 'b'
}
var target = {
  a: 'a',
  sub: sub // <= simulating a sub form
}
var source = {
  sub: {
    b: 'c'
  }
}
mergeDeep(Object.assign({}, target), source)
console.log(target.sub === sub) // true, this is supposed to be false

[awhillas]

Yep. Looks like that was it. Of course the only piece of code i didn't write tests for. Sorry for hassling you with this. Here is the revised function incase anyone is interested:

export default function mergeDeep(target, source) {
  let output = Object.assign({}, target);
  if (isObject(target) && isObject(source)) {
    Object.keys(source).forEach(key => {
      if (isObject(source[key])) {
        if (!target[key]) Object.assign(output, { [key]: {} });
        output[key] = mergeDeep(target[key], source[key]);
      } else {
        Object.assign(output, { [key]: source[key] });
      }
    });
  }
  return output;
}