Stop treating your ES modules like junk drawers

I wanted to talk about a pattern I see fairly often, and why I think it's problematic: module-level state in ECMA modules. This could be any state that is defined at the module level, and uses the module's scope as a means of achieving privacy. I'm only concerned with mutable state, constant values don't manifest the problems we'll explore below.

Together we'll look at an example that comes from some code I'm working on, and how to go about removing this pattern and improving the code overall.

This actual code isn't too hard to follow: getSlug returns a unique identifier, recalculating as necessary to avoid duplicates based on an in-memory cache.

To ensure that things work as expected, we should probably add a unit test!

This works reasonably well, at least for now. But it's worth considering that we're not testing the important parts yet.

One approach to supporting our testing might be to extend our mocking:

You might be able to see before even trying this code that it's going to get very messy very quickly: making assertions on a mock's call order and parameter list(s) is fine in certain cases, but it's not extensible or flexible enough to be the foundation of a test suite.

Even worse, these tests will only pass when run in this order! If we were to move the second test to be before the first we'd also have to change the mocking order, which is a good indicator that this whole organization needs more thought.

In search of something better

The next approach we might reach for is to expose the cache as a module-level export, and/or add a method to poke at it from our unit tests:

On the one hand, this would make testing much easier... we can manually manipulate the cache from the outside, which opens up all sorts of testing possibilities. On the other hand though, we've changed our public-facing contract for no external value — which might be ok but warrants a closer look.

At this point it's fair to step back and ask: why are we treating the ES module format like a stateful container?

What's actually wrong here?

The underlying problem with this code is that we're treating the module like an implicit object, when an explicit object would be better. By treating the module format as a container, we're reducing the clarity of our implementation in significant ways:

  1. The cache is reachable, but it's not clear that we don't want consumers to use it
  2. The reset method is also reachable, but it's not clear why it exists if the cache is also exposed
  3. We've mixed public and non-public details into a structure that is hard to reason about from the outside

It seems to me that the solution in this case is pretty clear: if we eliminate module-level state in favor of an exported object that contains all the relevant state, things become considerably easier.

Going back to our example with this strategy in mind, here's our revised approach:

We can use this to test the cache-miss and the collision cases, and tests can be re-ordered because they are no longer coupled to each other (which is itself a good sign that tests are well-organized and durable!):

Now these tests could be moved around, and/or we could cover many additional cases because there is a clear path to setting up necessary state and re-setting it between tests.

Wrapping up

In closing, be wary of using the module-format as a convenience for emulating privacy. This pattern adds complexity, reduces testability, and generally make your designs more brittle. You can achieve similar results with fewer headaches by reaching instead for more predictable and explicit approaches.

Thanks for reading!