This past weekend, I wrapped up the inline forms for Cushion’s new invoice form. As expected, laying this early groundwork paved the way for the rest of the inline forms, so I was able to knock one out each day. Since all of these forms were a substantial amount of work for one stretch, I didn’t want to jump right into another heavy task, so I decided to dedicate this week to cleanup.

As I build Cushion, I often keep a “cleanup” list alongside my to-dos. This list mainly consists of refactors that I could do in the moment, but decide not to because they might lead me down a rabbit hole. This list is also useful for snippets I might want to extract into a reusable piece of code, but there aren’t enough occurrences to justify it yet. I normally tackle one of these cleanup items at a time in between other work, but this time, I felt like batching them all for a single week. The most significant refactor I completed had to do with Cushion’s form states.

The initial form states in Cushion’s new form system started with a FormState object. This object kept track of whether the form had changed (for enabling the submit button) and whether the form was currently being saved (to disable the form, but also to show the saving animation). When I added error alerts to forms, I also included an error property on this object that would show a popover on the submit button.

interface FormState {
  error: Error | null;
  hasChanged: boolean;
  isSaving: boolean;
}

const formState:FormState = {
  error: null,
  hasChanged: false,
  isSaving: false,
}

Since this new work on Cushion is my first foray into TypeScript, I’m learning as I go, so the FormState started as an interface. Because of this, anywhere I needed the FormState, I would need to instantiate an object with the necessary properties—hasChanged, isSaving, and error. After creating a few forms, this repetition became a code smell. I try my best to keep Cushion’s codebase DRY (don’t repeat yourself), so I’m always looking for ways to avoid repetition. In this case, the FormState object became a prime candidate for refactoring.

class FormState {
  error: Error | null = null;
  hasChanged = false;
  isSaving = false;
}

Since the main problem with the FormState was the chore of setting the same default values each time, I decided to turn it into a class instead of an interface. This let me replace the objects with new FormState and set the defaults once, within the class. That was the easy part, but this refactor revealed other areas I could improve.

async handleSubmit (e:FormEvent):Promise<void> {
  e.state.error = null;
  e.state.isSaving = true;

  const [err, res] = await createInvoice(e.payload);

  if (res) {
    e.state.hasChanged = false;
    // alert success
    // change route
  } else if (err) {
    e.state.error = err;
  }

  e.state.isSaving = false;
}

In my forms, when I handle the submit action, I reset the FormState’s error, set the saving boolean to true, then make the request to the API, and finish by resetting the change boolean upon success or populating the error upon failure. I also reset the saving boolean regardless of success or failure. These steps are exactly the same with every form, but I was manually calling them in each submit handler, which stood out as another area I could clean up.

class FormState {
  async handle<T, E = Error> (promise: Promise<T>):Promise<[E | void, T | void]> {
    this.error = null;
    this.isSaving = true;

    const [err, res] = await promise;

    if (res) this.hasChanged = false;
    if (err) this.error = err;
    this.isSaving = false;

    return [err, res];
  }
}

I ended up creating an async handle method on the FormState class that wraps the request promise and sets the correct properties before and after the request. For those who aren’t too familiar with TypeScript, the T and E parts might seem confusing, but they’re simply “type variables” that let this method maintain proper type safety.

async handleSubmit (e:FormEvent):Promise<void> {
  const [, res] = await e.state.handle(createInvoice(e.payload));

  if (res) {
    // alert success
    // change route
  }
}

Refactoring the submit handler to use the FormState.handle method replaced the entire block with a single line of code. Now, I simply need to react to a successful request.

Being able to take the time to refactor code makes writing coding so much more enjoyable. It truly bothers me when I notice myself copy-pasting the same code several times throughout the codebase. It’s okay for trying something out, but once you do, take the time to clean it up. Back when Cushion was a month-to-month sprint, I didn’t feel like I had time to clean up because refactoring doesn’t feel like forward progress, but it is—just not immediate.

I know that all the care and consideration that I’m putting into Cushion’s new system will pay off massively years from now. Would-be bugs will be caught by tests or type checks. Updates to complex parts of the app will come with the safety net and confidence of those tests. Adding to the app will be much easier due to the reusability of everything. Knowing all of this gets me excited. I like to think that Future Jonnie will appreciate that I spent the extra time to make his life easier—or not. He might end up being a jerk.