bug in getTemplate for custom factory

[benmonro]

I have a custom factory in which I'm using getTemplate() to define a template for the factory, as mentioned here: https://github.com/gcanti/tcomb-form/issues/243

class MyFactory extends t.form.Component {

  getTemplate() {
    return (locals) => {
      return <MyComponent />;
    };
  }

}

However, it appears that locals.attrs.id & locals.attrs.name are not set in the locals parameter that is passed to the template. Other fields on the form are using a custom skin I wrote. Those fields _DO_ have the attrs properly set, even though I don't explicitly set them in the field options. I've even tried setting the 'attrs' field explicitly and it still does not get passed to the function which getTemplate() returns.

[gcanti]

Hi @benmonro, thanks for your PR. It's not a bug, it's by design. You can fix it using the decorators.attrs decorator...

@t.form.decorators.attrs
class MyFactory extends t.form.Component {

  getTemplate() {
    return (locals) => {
      return <MyComponent />
    };
  }

  // override the default getLocals methos in orded to pass the attrs
  getLocals() {
    const locals = super.getLocals();
    locals.attrs = this.getAttrs();
    return locals;
  }

}

...or extending a default factory

// no need to use the decorator here
class MyFactory extends t.form.Textbox {

  getTemplate() {
    return (locals) => {
      return <MyComponent />
    };
  }

  // no need to override the standard getLocals method here

}

[benmonro]

Hi @gcanti ok no problem, just out of curiosity why is this the design to exclude attrs from the Component?

[volkanunsal]

@gcanti Is it possible to add the custom factory in this thread into an examples folder? I think it's super interesting. I'd never seen a custom factory before.

[benmonro]

@volkanunsal I ended up creating a generic component factory that will work with any of our components. in your options for your field all that you need to do is say factory: ComponentFactory(MyComponent)

//ComponentFactory.jsx
export default (Component) => {

    @tcb.form.decorators.attrs
    class ComponentFactory extends tcb.form.Component {
        constructor(props) {
            super(props);
            this.onChange = this.onChange.bind(this);
            this.validate = this.validate.bind(this);
            this.state = {value: this.props.value};
        }

        validate() {
            return validate(this.state.value, this.props.type);
        }

        onChange(e) {
            this.state.value = e.target.value;
            this.props.onChange(this.state.value, this.props.ctx.path);
        }

        // override the default getLocals methos in orded to pass the attrs
        getLocals() {
            const locals = super.getLocals();
            locals.attrs = this.getAttrs();
            return locals;
        }

        getTemplate() {
            return (locals) => {
//field cell is a helper method we have that renders the label, errors etc for all of our components.
                return fieldCell(locals,
                    <Component
                        {...getExtraAttributes(locals)}
                        {...this.props.options}
                        onChange={this.onChange}
                        value={this.props.value}
                        />);
            };
        }
    }

    ComponentFactory.displayName = 'NuiTcombFactory';
    ComponentFactory.propTypes = {
        ctx: React.PropTypes.object,
        onChange: React.PropTypes.func,
        options: React.PropTypes.object,
        path: React.PropTypes.array,
        type: React.PropTypes.object,
        validate: React.PropTypes.func,
        value: React.PropTypes.object
    };
    return ComponentFactory;
}

[benmonro]

@gcanti If I do a PR that includes a ComponentFactory would you accept it? Seems like people might find it useful to be able to just simply override that factory, which wouldn't require them to use the attrs decorator or have to define getLocals(). Thoughts?

[gcanti]

why is this the design to exclude attrs from the Component?

@benmonro well actually a bad design. When I originally wrote the attrs decorator there was factories which didn't use it (namely Struct and List), so I kept it separated. Probably it was a mistake since the attrs decorator uses getId and getName and they are not decorators.

@volkanunsal I'll try to add more documentation on this subject but keep in mind that defining a custom factory should be a rare requirement, basically when you want to handle a new type (or perhaps when you want to pack a custom template + a custom transformer).

which wouldn't require them to use the attrs decorator or have to define getLocals(). Thoughts?

I think is more handy to just extend a default factory based on which type you want to handle or (much better) simply define a template.

[volkanunsal]

@gcanti Thanks. I think an examples folder would be good nonetheless because there are a ton of use cases that are not covered by the guide, and people keep opening an issue to ask them. If some of those code examples you include in your replies were in the examples folder, there might be fewer new issues about how to do this or that.