My first React 'aha' moment. Is this an antipattern?

Streaming on twitch my first project that uses react, that is not just following a tutorial or online course, is a little daunting but also exhilarating. A benefit of coding projects in the open is that I can more easily share the lessons I am learning.

From the online courses, I had learnt how React works, but I hadn't internalised some of the core concepts of building React projects. I am sure the information was there, it's just that my brain didn't have room to absorb it all.

Context

My first revelation happened when I started building a logging component for STDOUT and STDERR buffers - like you see on CI tools. I found a random open source example on Travis CI during the stream to see how the markup was put together:

I decided to make the assumption for my simple tool, that STDOUT text should be green and STDERR text should be red. I am in control of the executable being run so I can make silly decisions like this... at least for the time being.

Antipattern

My first attempt was to push styled spans (Stdout and Stderr) containing the text into an array stored in the state:

moku.stdout.on('data', data => {
  this.setState({
    output: this.state.output + <Stdout>data.toString()</Stdout>
  });
});

moku.stderr.on('data', data => {
  this.setState({
    output: this.state.output + <Stderr>data.toString()</Stderr>
  });
});

render() {
  return <Container>
    <pre>{this.state.output}</pre>
  </Container>
}

The alarm bells went off when I realised that I was storing HTML in the state, not data. I decided that I should think of React component state like a datastore, as I wouldn't pump presentational markup into a database (unless it's for a WYSIWYG field) because that restricts what you can do with it. If I store the raw content then I have more flexibility and re-usability.

Choose the path that leads to the most options.

Data-centric refactor

I decided to store an array of objects, each with a type property that I could set to stdout or stderr, and then use a ternary expression to choose the appropriate component for the object in the render function:

moku.stdout.on('data', data => {
  this.setState({
    output: this.state.output.concat({
      type: 'stdout',
      data: data.toString()
    })
  });
});

moku.stderr.on('data', data => {
  this.setState({
    output: this.state.output.concat({
      type: 'stderr',
      data: data.toString()
    })
  });
});

render() {
  return <Container>
    <pre>
      {this.state.output.map(
        (obj, index) =>
          obj.type === 'stdout' ? (
            <Stdout key={index}>{obj.data}</Stdout>
          ) : (
            <Stderr key={index}>{obj.data}</Stderr>
          )
      )}
    </pre>
  </Container>
}

Summary

This example is short but it demonstrates what I think of as an antipattern: don't store HTML in the components state. If you disagree then don't stay silent, I am new to React and would appreciate pointers! Any feedback on the code would be appreciated.

For my streaming experiment, this helped with writing this article a ton! I was able to scrub back through the video and view the code I had written leading up to the git commit.

Follow along

Reading this article also saved you from watching my mumbling stream - I am focussing on getting better at presenting live coding. If you want to follow along with my React/Electron progress I mainly steam on Mondays but I'll write up any lessons I've learned here anyway :)

https://www.twitch.tv/joejamesio

Comments

[Jeffrey Tao]

For the general pattern, you've got it right. HTML is a render-time concern, and your state should be more concerned with pure data. One of the key benefits of React is that you no longer need to manually inspect and manipulate HTML.

One detail:

    {this.state.output.map(
        (obj, index) =>
          obj.type === 'stdout' ? (
            <Stdout key={index}>{obj.data}</Stdout>
          ) : (
            <Stderr key={index}>{obj.data}</Stderr>
          )
    )}

You want to be careful about how you specify keys when returning an array of components. From the React Docs:

When you don’t have stable IDs for rendered items, you may use the item index as a key as a last resort ... We don’t recommend using indexes for keys if the order of items may change. This can negatively impact performance and may cause issues with component state.

They key needs to uniquely identify a component among its siblings. In this case, as long as your output scrollback grows indefinitely, keys do uniquely identify elements (since they basically correspond to a unique counter of log lines), but if you were to do something like limit scrollback to the last N lines, what would happen to the element at index 0? As soon as you add an N+1th line, the element at index 1 is now index 0, and we've violated the unique key rule because a component has changed keys.

That's a comparatively small nitpick, though. The before/after you've written in the data-centric refactor is definitely a large step in the right direction.

[Joe]

I did read about that in the docs and dismissed it as something I didn't need to worry about for this scenario, but as you've pointed out, I could end up running into this issue if I implemented a max size on the output. So thanks for highlighting this to me, nitpicking is helpful :)

Hmmm, so I guess to get a unique key in this scenario I could increment an id counter when appending objects to the output collection, like a primary key. The db analogy seems to help me figure this stuff out lol.

On the note of nitpicking: I turned on some linters yesterday and got a bunch of warnings, so I've found a few ways to improve the code in the example. I think the main one is not accessing state when trying to update it, so I've moved concatenating the output collection to a callback:

moku.stdout.on('data', data => {
  if (data.length > 0) {
    this.setState(prevState => ({
      isRunning: true,
      output: prevState.output.concat({
        type: 'stdout',
        data: data.toString(),
      }),
    }));
  }
});

I've also learnt about destructing arguments. These linters are very educational :p

[Art]

I'd say what your original implementation was not actually as much of an antipattern as you'd think.

What you were storing inside of state was not actually HTML, but React Elements.

React Elements are what is returned from React.createElement, which is what angle braces desugar to in JSX.

Since, I'm assuming, the source of STDOUT and STDERR will always be rendered by their corresponding components, I think this is a fine solution.

I've seen elements stored in state in many components. We use it in prod for stuff like modals for example...

[Joe]

This is very interesting. Thank you for pointing this out to me.

I see now that in both examples I am storing objects in the state. The first example is using JSX objects and the second example is using my own JSON objects.

In my second example, my own objects may be easier to manipulate across multiple rendering scenarios, but if that is not a requirement then it's additional work for the browser to transform these into JSX objects, and this output stream could theoretically stack up to contain thousands of objects.

So I agree with you, this isn't an antipattern. I am now thinking of this as a refactoring technique for re-usability that can be adopted when multiple components need to render centralized data.

Thanks for providing the example of storing JSX for modals in the state, that is a great example that is a common project requirement.

[Art]

Glad you found that useful!

Perhaps one more thing to add, is that if you were to serialize the state (to persist it in local storage for example), you'd also have to use custom objects. React Elements are circular, so you can't easily stringify them.