DEV Community

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

Joe on August 20, 2018

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 exhilar...

Read full post

Jeffrey Tao • Aug 20 '18

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 • Aug 21 '18

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 • Aug 21 '18

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 • Aug 22 '18

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 • Aug 22 '18

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.