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.

import randomstring from 'randomstring';
const slugs = new Set();
export const getSlug = () => {  const attempt = randomstring.generate({    length: 4,    charset: 'alphabetic',    capitalization: 'uppercase',  });
  // Collision! Roll the dice again  if (slugs.has(attempt)) {    return getSlug();  }
  // Remember our unique value to avoid future collisions, then return it  slugs.add(attempt);  return attempt;};

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

// This code assumes Jest, but should be applicable to any test runnerimport { getSlug } from './getSlug';import { randomstring } from 'randomstring';
jest.mock('randomstring', () => {  return {    generate: jest.fn().mockImplementation(() => {      return 'ABCD';    })  };});
describe('getSlug', () => {    it('should return a slug', () => {    const actual = getSlug();
    // Assert that we called the backing method with the expected params, and    // that the value was properly returned    expect(randomstring.generate).toHaveBeenCalledWith({      length: 4,      charset: 'alphabetic',      capitalization: 'uppercase'    });    expect(actual).toBe('ABCD');  });});

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:

// This code assumes Jest, but should be applicable to any test runnerimport { getSlug } from './getSlug';import { randomstring } from 'randomstring';
jest.mock('randomstring', () => {  return {    generate: jest.fn()      // Manually stub each call, in numerical order      .mockImplementationOnce(() => {        return 'ABCD';      })      .mockImplementationOnce(() => {        return 'ABCD'; // simulate a collision      })      .mockImplementationOnce(() => {        return 'WXYZ'; // simulate re-requesting as a solution      })  };});
describe('getSlug', () => {    it('should return a slug', () => {    const actual = getSlug();
    // Assert that we called the backing method with the expected params, and    // that the value was properly returned    expect(randomstring.generate).toHaveBeenCalledWith({      length: 4,      charset: 'alphabetic',      capitalization: 'uppercase'    });    expect(actual).toBe('ABCD');  });
  it('should return a different slug if there is a collision', () => {    const actual = getSlug();
    // Assert that we called the backing method 2 times, and that the second    // result was returned    expect(randomstring.generate).toHaveBeenCalledTimes(2);    expect(actual).toBe('WXYZ');  });});

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:

import randomstring from 'randomstring';
// `export` to expose outside the module, and `let` so that we can re-assignexport let slugs = new Set();
export const getSlug = () => {  const attempt = randomstring.generate({    length: 4,    charset: 'alphabetic',    capitalization: 'uppercase',  });
  // Collision! Roll the dice again  if (slugs.has(attempt)) {    return getSlug();  }
  // Remember our unique value to avoid future collisions, then return it  slugs.add(attempt);  return attempt;};
// Maybe expose a testing hookexport const reset = () => {  slugs = new Set();};

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:

The cache is reachable, but it's not clear that we don't want consumers to use it
The reset method is also reachable, but it's not clear why it exists if the cache is also exposed
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:

import randomstring from 'randomstring';
// Everything in a single object, completely controllable from the outside!export const slugs = {  cache: new Set(),
  get: () => {    const attempt = randomstring.generate({      length: 4,      charset: 'alphabetic',      capitalization: 'uppercase',    });
    // Collision! Roll the dice again    if (slugs.cache.has(attempt)) {      return slugs.get();    }
    slugs.cache.add(attempt);
    return attempt;  },
  reset: () => {    slugs.cache = new Set();  },};

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!):

// This code assumes Jest, but should be applicable to any test runnerimport { slugs } from './getSlug';import { randomstring } from 'randomstring';
describe('get slug', () => {  // Zero out our state before each test  beforeEach(() => {    slugs.reset();  });
  it('should return a slug', () => {    randomstring.generate.mockImplementationOnce(() => 'ABCD');
    const actual = slugs.get();
    // Assert that we called the backing method with the expected params, and    // that the value was properly returned    expect(randomstring.generate).toHaveBeenCalledWith({      length: 4,      charset: 'alphabetic',      capitalization: 'uppercase'    });    expect(actual).toBe('ABCD');  });
  it('should return a different slug if there is a collision', () => {    slugs.cache.add('ABCD');
    randomstring.generate      // ..collision, will trigger the next call which is a...      .mockImplementationOnce(() => 'ABCD')      // ...non-collision      .mockImplementationOnce(() => 'WXYZ');
    const actual = slugs.get();
    // Assert that we called the backing method 2 times, and that the second    // result was returned    expect(randomstring.generate).toHaveBeenCalledTimes(2);    expect(actual).toBe('WXYZ');  });});

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.

What about privacy and/or ownership concerns?

A common motivator for module-level state is "Information-hiding", which is a strategy used to ensure that consumers aren't able to access internals. In my experience however, the trade-offs are rarely worth it and overall maintainability goes up when choosing a different strategy:

Private class fields are now part of the ECMA specification and are natively available in newer browsers. If you're using a transpilation step, chances are your bundler has configurable support as well.
private is a supported access modifier in Typescript, and while it only works in typed code it is very easy to get started with.
In the ancient days of Javascript development, a leading _ was used to signal to consumers that a field might be reachable, but it was intended to be off-limits by the author. There is no actual enforcement of course, but the convention is commonly understood.

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!