Seeking a patterns to avoid "mutation-after-consumption"

Hello!

I’m seeing the following warning in my app: DEPRECATION: You attempted to update "fields" on "FormComponent", but it had already been used previously in the same computation...

It relates to a form component, which has a @tracked ‘fields’ object in order to keeps tabs on which fields are being rendered into the form, and their status.

Each field has access to the form’s setupField and validateField actions. When a field is initiated, it uses setupField to register with the form. On input, the form then handles validation and tracks the state of each field, and the validation of the form itself (eg, the form is invalid until all registered fields pass their validations).

A basic implementation looks like this:

<Form::Base as |form|>
  <form.Body as |body|>
    <Fields::String::Edit
      @value={{this.email}}
      @type="email"
      @label="Email Address"
      @options={{hash
        validators=(array "email" "required")
        onSetup=body.setupField
        onValidation=body.validateField
      }}
    />
  </form.Body>
  <form.Actions/>
</Form::Base>

An extract of the Form component looks like this:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { isEmpty } from '@ember/utils';

export default class FormComponent extends Component {
  @tracked fields = {};

  @tracked formIsInvalid = true;

  constructor(owner, args) {
    super(owner, args);
    this.buttonText = this.args.buttonText;
    this.fieldValidators = {
      // example - evaluates and returns any error messages
      required: (inputValue) => {
        if (isEmpty(inputValue) || !inputValue) {
          return 'Required.';
        }
      },
    };
  }

  @action
  setupField(field) {
    const newField = {
      previousValue: field.args.value,
      errorMessages: [],
      isValid: true,
    };
    this.fields = { ...this.fields, [field.id]: newField };
    this.validateField(field);
  }

  @action
  validateField(field, inputValue) {
    const { id } = field;
    const { fields } = this;
    const previousValidity = fields[id].isValid;
    let errorMessages = [];
    if (field.options && field.options.validators) {
      errorMessages = field.options.validators
        .map((v) => this.fieldValidators[v](inputValue || field.args.value, field.args.options))
        .filter(Boolean);
    }
    fields[id].errorMessages = errorMessages;
    field.errorMessages = fields[id].errorMessages;
    fields[id].isValid = fields[id].errorMessages.length === 0;
    field.isValid = fields[id].isValid;
    this.fields = fields;
    this.formIsInvalid = Object.values(this.fields).some((i) => !i.isValid);
    if (previousValidity !== field.isValid && this.args.onValidityChange) {
      this.args.onValidityChange(this.formIsInvalid);
    }
  }
}

What I have works but clearly isn’t supported by Ember. I’m looking for any guidance on Octane best-practice patterns, for forms / fields / validations, where I can retain a high level of control over what’s rendered into the form. I also want the form to be able to report its validity to a parent component too - for example, in an instance where the body of the form is rendered into a modal, and I want to be able to tell that component whether or not to display a submit button. this.args.onValidityChange(this.formIsInvalid) handles that in the above component, but is that going to create the same issue moving forwards?

Cheers :crossed_fingers:t2:

So this won’t necessarily solve your problem just yet but I just wrote something with a very similar need. I chose to write it so you pass one or more “fieldsets” (each with a model, so it supports multiple models) to the component and it constructs form controls and changesets and validations and then yields out not only the form controls but also buttons for submit/cancel (for use in modal scenarios like you describe above for example, then the child doesn’t have to provide any state to its parent). The form controls and buttons are “pre-wired” so rendering them is very simple. But it doesn’t sacrifice flexibility because the pre-wired form controls and buttons can be customized completely.

So put a little differently, this component takes as input details about the form fields (type, model attribute, validations, etc) and uses it to construct changesets (with validations) and control components (“pre-wired” with getters/setters/validations/etc) and as “output” it yields the controls and the buttons “ready to go” or “ready to customize” depending on your needs. Thus the component args determine the business logic portion of the form and how it functions, and the forms block determines only “how the form should be rendered”.

Anyway now i’m just rambling so maybe an example invocation would make more sense…

<FormBuilder>
  @fieldsets={{hash
    user=(hash
      model=@user
      fields=(hash
        firstName=(hash type="string" minLength=3 required=true)
        lastName=(hash type="string" minLength=3 required=true)
      )
    )
  }}
  as |form|
>
  <div class="flexy-flex">
    <form.fields.user.lastName />
    <form.fields.user.firstName />
  </div>

  <form.buttons.cancel />
  <form.buttons.submit />
</FormBuilder>

In this example you see that the “fieldset” is being passed in and contains all the information the component needs to build the form controls and the validations. You may also notice that the controls can be rendered in any arbitrary order and/or wrapper inside the block. The fieldsets can also be defined on the backing class too. Here’s a more complicated example:

// components/some-form.js
import Component from '@glimmer/component';

const instagramValidator = ({ requirePrefix } = { requirePrefix: true }) => {
  return (_key, newValue/*, oldValue, changes, content*/) => {
    if (newValue && newValue.match && newValue.match(`^${requirePrefix ? '(http|https):\/\/' : ''}(www\.)?(instagram\.com|instagr\.am|instagr\.com)\/[A-Za-z0-9_\\-.]+(\/)?$`)) return true;
    return 'bad instagram url';
  }
}

export default class SomeFormComponent extends Component {
  fieldsets = {
    user: {
      model: this.args.user,
      fields: {
        firstName:     { type: "string", label: "First Name", required: true, minLength: 2 },
        lastName:      { type: "string", label: "Last Name", required: true, minLength: 2 },
        dateOfBirth:   { type: "date", label: "Date of Birth", required: true },
        ssn:           { type: "number", label: "SSN", integer: true, length: 9 },
        zipCode:       { type: "number", label: "Zip Code", integer: true, positive: true, length: 5 },
        maritalStatus: { type: "radiogroup", label: "Marital Status", required: true,
          options: [
            {value: "single", label: "Single" },
            {value: "married", label: "Married" },
            {value: "complicated", label: "It's Complicated" }
          ]
        }
      }
    },
    userStuff: {
      model: this.args.userStuff,
      fields: {
        website: { type: "url", label: "Website" },
        instagram: {
          type: "string",
          label: "Instagram"
          validator: instagramValidator
        }
      }
    }
  }
}

// components/some-form.hbs
<FormBuilder @fieldsets={{this.fieldsets}} as |form|>
  <div>
    <form.fields.user.lastName />
    <form.fields.user.firstName />
  </div>
  <form.fields.userStuff.website />
  <form.fields.userStuff.instagram />
  <form.fields.user.zipCode />
  <form.fields.user.dateOfBirth />
  <form.fields.user.maritalStatus as |RadioGroup options|>
    <RadioGroup as |rg|>
      {{#each options as |option|}}
        <rg.radio @label={{option.label}} @value={{option.value}} />
      {{/each}}
    </RadioGroup>
    <UI::ContextualNote @label="marital status is required for tax purposes." />
  </form.fields.user.maritalStatus>

  <form.buttons.cancel />
  <form.buttons.submit />
</FormBuilder>

In this example you can see the “maritalStatus” form control is being “cracked open” and customized by rendering the options in a custom format and adding a “contextual note”. There’s also a custom validator function being applied to the instagramUrl field.

Anyway… I’m not sharing this to say you should do something like it or to say that what I wrote is a great abstraction or the “right way”, it’s more to try to emphasize one of my chief considerations when writing it which is: data ownership and “flow”. I wanted to try to conform to DDAU as much as possible and to make sure that the data flow was clear. The “outer” component provides fieldset(s) and model(s), passes them into FormBuilder, FormBuilder creates the changesets and form control components, and then the block determines how to render the form. The business logic occurs inside FormBuilder.

Your abstraction is very similar in a lot of ways, but it looks like parent and child have fuzzier boundaries and are sort of “reaching into” each other, or needing each other’s state. I think that sort of situation is when you tend to see issues crop up with autotracking state. So I guess in summary my recommendation would be to do your best to find an abstraction where “children” are only using actions to manipulate state based on user interaction and not as part of the “setup” of the form. It’s too easy to get into scenarios where parent action gets fired a bunch by children which mutates parent state which rerenders children and so on…

Hope that’s clear enough to be helpful and not just a lot of word barf :laughing:. Happy to share more details about “FormBuilder” if that would be helpful.

This is extremely helpful, thank you so much for taking time to write such a considered response. What you have said about the fuzzier boundaries is absolutely right, and the approach you’ve outlined gives me a great place to start. Is FormBuilder in a public repo that I can take a look at?

Unfortunately it’s not and I’d consider extracting it into an addon but the difficulty is that the component that renders the form controls is hard-baked for our UI library and I’m not sure how easy it would be to allow BYOUI (especially looking towards static template imports, etc).

Anyway I’d be happy to share more of the details. Not sure if it would be more helpful to give you a general overview of the architecture for starters or just a bunch of code…

The weirdest part is how it creates the form control components (which we call “form group” because we’ve still got some vestiges of bootstrap). To yield them out in a “contextual” format we need a CurriedComponentDefinition but there is no javascript API for creating those yet, so I had to do some slightly questionable things using the (component ...) template helper. It totally works and seems elegant enough in a nasty way…

  {{#each-in @fieldsets as |modelName fieldset|}}
    {{#each-in fieldset.fields as |modelProp details|}}
      {{call (fn this.buildFieldComponent
        modelName
        modelProp
        (component "u-i/form-group"
          model=(get this.changesetsHash modelName)
          modelProp=modelProp
          label=details.label
          options=details.options
          required=(or details.required details.requiredWhen)
          type=details.type
          extraArgs=(hash
            min=details.min
            max=details.max
          )
        )
      )}}
    {{/each-in}}
  {{/each-in}}

(note: if your forms only deal with one model at a time you could simplify this by only having one “fieldset” and one model per form)

Anyway… what that ^ does is loop over the “fieldsets” and then of course the “fields” within each fieldset and use that information to create a CurriedComponentDefinition via the (component ...) helper, then it passes the CCD to an action where it is stashed in an object hash called fieldComponents. Then later in the template it’s yielded out along with some other fun stuff:

  {{#let
    (component "u-i/button" label=... onClick=(perform this.rollback this.changesetsHash))
    (component "u-i/button" label=... type="submit" theme="primary" disabled=(or this.allPristine (not this.allValid)))
    as |CancelButton SubmitButton|
  }}
    {{yield (hash
      fields=this.fieldComponents
      buttons=(hash cancel=CancelButton submit=SubmitButton)
      changesets=this.changesetsHash
      actions=(hash submit=this.submit rollback=this.rollback)
    )}}
  {{/let}}

Most of the rest of FormBuilder is just constructing changesets and validators from the fieldsets and then handling the submit and rollback actions which is fairly straightforward, but also sort of implementation specific for us.

I don’t love the UI::FormGroup component we have but it’s necessary to get this all working. Basically all it does is given some params (as you can see above) it renders a label and form control which is “pre-wired” to get/set value from the model. There’s a little more to it than this but the bulk of the template looks something like this:

  {{#let
    (unique-id)
    (if (and @model @modelProp) (get @model @modelProp))
    (set (or @model (hash noop="")) (or @modelProp "noop"))
    (if (and @model @modelProp) (invoke "validate" @modelProp @model))
    as |uniqueId controlGetter controlSetter controlValidator|
  }}
    {{!-- render label bit here, excluded for brevity --}}

    {{#if (includes @type (array "string" "text" "phone" "tel" "email" "url" "date"))}}
      {{#let
        (component "u-i/input"
          type=(get (hash string="text" text="text" tel="tel" phone="tel" email="email" url="url" number="number" date="date") @type)
          value=controlGetter
          onChange=controlSetter
          onBlur=controlValidator
          uniqueId=uniqueId
        ) as |FormControl|
      }}
        {{#if (has-block)}}
          {{yield FormControl uniqueId}}
        {{else}}
          <FormControl />
        {{/if}}
      {{/let}}
    {{else if (eq @type "datetime")}}
      {{#let (component "datetime-picker" date=controlGetter onChange=controlSetter uniqueId=uniqueId) as |FormControl|}}
        {{#if (has-block)}}
          {{yield FormControl uniqueId}}
        {{else}}
          <FormControl />
        {{/if}}
      {{/let}}
    {{else if (eq @type "radiogroup")}}
      {{#let (component "u-i/radio-group" value=controlGetter onChange=controlSetter uniqueId=uniqueId) as |FormControl|}}
        {{#if (has-block)}}
          {{yield FormControl @options uniqueId}}
        {{else}}
          <FormControl as |radioGroup|>
            {{#each @options as |option|}}
              <radioGroup.radio @label={{option.label}} @value={{option.value}} />
            {{/each}}
          </FormControl>
        {{/if}}
      {{/let}}
    {{else if ...}}
      ...
    {{/if}}
  {{/let}}

  {{!-- render validation errors here, excluded for brevity --}}

So yeah… kinda gross, extremely verbose, but it allows us to do a lot of cool stuff.

So those are some of the weirder but more important parts, happy to share more about any of that if you’d like.

1 Like

Thanks again for the generous feedback. Lots to digest here so I’m going to take a deeper look this afternoon and start thinking how I can apply some of the thinking to our own situation. :+1:t2:

Useful discussion, thanks, I appreciate that! :+1:

I deleted my previous post because of some of the issues I came up against when trying to implement the original design for our Form component. I couldn’t find a way to edit that post and didn’t want people to walk into the same walls I built for myself. Having addressed most of the problems (with one fundamental issue left to tackle - see part 4, below) I thought I’d share the new approach in a fresh update.

The current implementation comes in four parts:

  1. a FormField class which is used to define entires in a fieldSet, which is passed into the form component:
// FieldSet class definition

import { tracked } from '@glimmer/tracking';
import fieldValidators from './field-validators';

export default class FormField {
  key;

  @tracked value;

  validators = [];

  validationOptions = {};

  constructor(args) {
    const { key, value, validators, validationOptions } = args;
    this.key = key;
    this.value = value;
    this.validators = validators;
    this.validationOptions = validationOptions;
  }

  get errorMessages() {
    return (this.validators || [])
      .map((v) => {
        const validator = fieldValidators[v];
        return validator(this.value, this.validationOptions);
      })
      .filter(Boolean);
  }

  get isValid() {
    return this.errorMessages.length === 0;
  }
}

// contact-form/controller.js
// instances of FormField are created as entries in a fieldSet POJO

fieldSet = {
   name= new FormField({
      key: 'name',
      value: this.args.model.name,
      validators: ['required'],
   })
   email= new FormField({
      key: 'email',
      value: this.args.model.email,
      validators: ['required', 'email'],
   })
}
  1. a Form component, which takes @fieldSet as an argument and yields it into its body:
{{! contact-from/template.hbs }}
{{! Example Form invocation }}

<Form
  @onValidityChange={{@onValidityChange}}
  @fieldSet={{this.fieldSet}} as |form|
>
   <form.Header />
   <form.Body as |body|>
      {{#let body.fieldSet as |fields|}}
         ...
      {{/let}}
   </form.Body>
   <form.Actions />
</Form>

The form has a formIsInvalid getter, which iterates over the entires in @fieldSet, checks their validity and reports to the form’s submit button and any parent component, where an @onValidityChange action has been passed in. This allows for submit buttons to be enabled / disabled in the parent context.

Unfortunately, the current implementation means that @onValidityChange is called more often than required. It would be nice to store previousValidity somewhere and compare it to currentValidity. I’m unsure how to do this without “introducing side effects”?

// components/form.js
// iterates over @fieldSet and checks validity

get formIsInvalid() {
  const currentValidity = Object.values(this.args.fieldSet).some((i) => !i.isValid);
  // would be nice to compare with previousValidity here
  if (this.args.onValidityChange) {
    this.args.onValidityChange(currentValidity);
  }
  return currentValidity;
}
  1. a FieldBase component which wraps around our inputs, consumes a @formField (which is an entry from the fieldSet), and provides access to validity and error messages.
{{! components/my-input.hbs
{{! MyInput component - example of how FieldBase wraps a basic input }}

<FieldBase
  @value={{@value}}
  @label={{@label}}
  @formField={{@formField}}
  @options={{@options}} as |field|
>
  <label for={{field.id}}>{{@label}}</label>
  <Input
    @value={{readonly @value}}
    @type={{@type}}
    id={{field.id}}
    placeholder={{@placeholder}}
    autocomplete={{@autocomplete}}
    disabled={{@disabled}}
    required={{@required}}
    {{on "focus" field.onTouched}}
    {{on "input" this.onChange}}
  />
</FieldBase>
{{! contact-from/template.hbs }}
{{! putting it together - a example Form invocation, using MyInput }}

<Form
  @onValidityChange={{@onValidityChange}}
  @fieldSet={{this.fieldSet}} as |form|
>
   <form.Header />
   <form.Body as |body|>
      {{#let body.fieldSet as |fields|}}
        <MyInput
          @value={{@model.name}}
          @formField={{fields.name}}
          @onChange={{fn this.setProperty @model "name"}}
          @type="text"
          @required={{true}}
          @label="Customer name"
          @options={{hash isTouched=body.showErrorsOnInit}} 
        />
...
  1. a setProperty action. I’m really unhappy with this bit and would welcome any ideas about how to overcome the challenge. The problem: using the name field as an example, it is not sufficient to update this.args.model.name, on change. We also need to update fieldSet.name.value to trigger the validation and error messages getters on the FormField instance. As a result we have to pass in a setProperty action that handles both updates:
// contact-form/controller.js 

// ideal
@action
setName(value) {
   this.args.model.name = value;
}

// actual
@action
setProperty(target, property, value) {
  target[property] = value;
  this.fieldSet[property].value = value;
}

It needs to be replicated everywhere a fieldSet is defined. It also means the @onChange passed into the input is quite verbose. Not nice. The ideal solution would be for the value in each FieldSet instance to contain a reference to the original, so that when this.args.model.name is set the getters are triggered. Is that realistic given the approach I’ve taken? What would a solution look like? Any ideas or examples would be greatly welcomed.

This is an imperfect solution but one that works for us today. I think I will continue to revise it as I understand more about Ember’s reactivity model.

Thanks again to @dknutsen for sharing all the great information about FormBuilder!

1 Like