RxJS Bugs Always Have Solutions

Even the best of us write bugs. This guy did it on purpose.

I’m going to point out a bug in Ben Lesh’s code which ended up just being semantics (more on this below).

Ben’s Original Tweet

Ben Lesh posted some code on Twitter for a talk he’s going to be doing at ngconf:

He wrote this code to provide a simple example where you can add to a number, undo, and redo in the DOM with very little code:

The goal is to both explain RxJS and show how, with only a few operators, you can do so much. This is also why he didn’t choose to render DOM elements using either Angular or React. It would significantly add bloat.

The Dreaded Bug

So where’s the bug? Press “+1” a couple times, you get 2 . Then press “Undo”. You should see 1 as you expect, but if you press “+1” again, it’ll show 3 instead of 2 .

After looking at the code, I found the issue. The reducer he provided to scan keeps accumulating values without any regard to the undo and redo actions:

There are a few ways I could think of fixing the issue, but I wanted to keep it simple.

Let’s also treat this like a professional project. We’re Ben’s editors. We found a bug in his code, and we’re getting paid to fix it in a way that it keeps with his original requirements.

From his tweet, our requirements are to have a “very, very simple undo operator” and only use merge , scan , filter , and map .

Fixing the Bug

Here’s my first go at a fix:

While this did fix the bug, my solution still ended up being too intense for his talk:

Remember, we’re supposed to be keeping in the guidelines of “very, very simple” and only 4 operators. Now we have two startWith operators and an added switchMap .

You wouldn’t need to explain startWith unless you have two of them. Now the order of operators has to be explained where before, he could probably leave it unexplained.

On top of that, I broke out the render pattern into a Subject called render$ . Now I’ve introduced subjects in the mix. Instead of everyone knowing already how observables work, now he has to explain what an observable is, how that’s different from an observer, and how a Subject is their combination.

What do we do now?

Another Attempt

Remember, we’re getting pretend paid here so we need to be professional about it.

Instead of complicating our operators, let’s check if we can simply improve the undo operator:

To me, this doesn’t look like a “very, very simple undo operator”, but considering what it does, this is about as simple as it gets. That means we could add onto it and stand to lose one “very”.

It’s written in TypeScript, and it’s returning an observable that combines the value of the undo, redo, and source observables. source is the observable containing the output value after it’s run through the buggy observable: source$ .

Also notice, this operator returns and observable with a scan on it. Whoa, scan’ception! That means this undo operator is also acting as a reducer. Because of that, it contains state, and we can use that state to pull some shenanigans.

The only reason we can do this is because the buggy output value from the previous scan will come into this scan . From here, we can hack around it.

I’d prefer not to use this method, but since we have some strict requirements and since I’ve already started writing the article, it’s worth trying.

In the undo ’s scan operator, state is an array of all values that came from source$ .

Anytime a new value comes in, we can store the difference in this reducer and subtract it out each time.

Hacky. But it works! And it was pretty simple to change.

Here’s the new code for undo :

We added a subtractor to the reducer.

When pushing to state, we’re now doing x - subtractor . Anytime we put in a new value, we’ll always be subtracting out the incorrect amount from the previous scan .

Anytime an undo occurs, we add the difference of the undid value and the new value to our subtractor .

When a redo occurs, we still subtract the undid value from the current value, but then we remove that difference from the subtractor .

While this does show some TypeScript errors, the code compiles. It fixes the bug and fits within our requirements.

Problems

The downside to this solution is its hackiness. It also couples our undo operator with a bug in another part of the code. That means our undo operator can’t be used anywhere else but in this particular system.

And that’s the problem with keeping things simple. Sometimes you need to add a few operators to make RxJS both more-powerful and easier to use.

I would never ever recommend anyone use the second fix because of how finicky it makes your codebase. Even though it solves the issue, it’ll create many other problems.

What Matters to Ben?

For the talk, what’s more important? Using only 4 operators, having a “very, very simple undo operator”, or having bug-free code?

That’s the most-important thing to think about.

It could very well be that this isn’t a bug. I’m guessing Ben wanted it this way to keep the code simple. We see it as a bug because we have an expectation of how “undo” and “redo” work. He knows this code is going to be in a talk so he wouldn’t click “add” again after clicking undo . Simple.

That’s what matters most. Figuring out what’s important for the business; not what you think is best for the code, or your own agenda. Don’t be an evangelist and fix his code for him when it doesn’t appeal to his needs unless you wanna have some fun like I did. :D

Redux-Observable FTW!

Because I see the Redux-Observable style as the future of clean code, I completely rewrote his entire example in that style. That means none of his original code, no TypeScript, the vertical coding style, and no bugs!

Each observable is in a function called an epic which provides an action$ observable. Instead of listening directly to other observables, we only have to listen to action$ .

When action$ updates, all our observables fire, and we use ofType to determine if the action is something we care to process. If so, push it through the pipeline.

Instead of reducers going into other reducers, I made a single reducer (could’ve made it two) which reduced a lot of the complexity of that old undo function. Similar to my first bug fix in the original code, I’ve dedicated a separate observable for rendering new output values to the DOM.

You can play around with it here:

My version looks very different from what Ben wrote, but I didn’t change the view, only the business logic behind it. I’m sure a lot of you have had to refactor code just like this before.

In a lot of my projects, I use Redux and don’t use component state at all. Because of this, I can spend time working on reducers and epics and leave the views alone. It also means I can separate views entirely from core business logic.

Why would I do that? Because I can write the same Redux logic and use it in either AngularJS or React at the same time in the same app. It’s not for everyone, but it worked for us in my last two teams.

Conclusion

What matters most are the business requirements. While you might have some inherent knowledge of how you think things should work, the people selling your product come first.

In this case, Ben Lesh is selling this product at a talk (EDIT: not this particular operator, just operators in general). I’m writing an article about how to fix bugs in someone else’s RxJS code. His requirements are completely different from mine.

Because of this, I believe Ben’s not going to fix the code. I can bet he won’t fix the bug because, as I said earlier, it’s probably not a bug for him.

EDIT

Ben Lesh provided clarification about the undo operator:

He really meant to call it showPreviousValues instead of undo . This is what I was talking about.

More Reads

If you liked what you read, please checkout my other articles on similar eye-opening topics: