Jest: Clearing the jsdom environment

[vvo]

Currently between two it() calls, any DOM node will stay, so you have to manually clean it up.

Should we:

• provide a way to easily clean the jsdom env? (besides removeChild or innerHTML, is there any jest.clearDOM()?)
• automatically clean it for the user between to it() (as an option)
• do nothing, just clean your own mess

Also relevant, when I used mocha, I was using this very simple tool: https://github.com/rstacruz/jsdom-global

[cpojer]

Cleanup doesn't really do much special stuff: https://github.com/rstacruz/jsdom-global/blob/master/index.js#L49

I suggest if you need this, to either:

• Write your own cleanup handler.
• Create a separate test file.

Does this work for you? At this point I'm quite hesitant to adding new APIs to support jsdom better.

[vvo]

Wfm

[cameron-martin]

It seems to me that it'd be best for jest-environment-jsdom to create a new instance of JSDOM for each test. This way less state is maintained between tests.

Is there any reason this isn't done currently?

[cpojer]

We do create new instances of jsdom, however we do not load jsdom from scratch which would increase runtime for each test by more than 100ms.

[cameron-martin]

That's a shame that instantiating jsdom is so expensive. I'll open an issue on the jsdom repo for discussion on either:

• Decreasing this startup time.
• Providing a cheap way of resetting jsdom's state.

[granmoe]

Just chiming in to say that this would be an absolutely amazing feature to have for the already incredible library that is Jest.

On the other hand, perhaps we should petition for a solution over in the jsdom repo?

[granmoe]

(Assuming perf issues could somehow be alleviated, of course.)

[vvo]

For what it's worth, last time we needed that we used:

  beforeEach(() => {
    document.body.innerHTML = '';
  });

[cameron-martin]

The issue with doing this is that you have to _know_ which state is modified during each test, which leaves a lot of room for human error.

[ryx]

The problem with simply clearing the "physical" DOM (i.e. innerHTML) is also that the shared state for virtual objects (window.*) still remains. This can result in very subtle errors (e.g. a single test running fine on its own but failing when running in a suite). This seems to be a common problem when using Object.defineProperty on globals like localStorage, navigator.*, etc..

I'd really vote for a more convenient way in jest-environment-jsdom that allows to reset the entire JSDOM on demand (e.g. something like jsdom.reset()). That way it would be possible to explicitly accept the performance penalty and have a reliable way to clean up.

[cameron-martin]

A configuration option for jest-environment-jsdom would work.

[granmoe]

It occurs to me that there's a point at which integration tests are close enough to end-to-end tests that they should just be made into E2E. However, this doesn't apply to the situations in which we're seeing this JSDOM issue on our team. Plus, there are often good reasons to do certain things as integration tests via jest instead of E2E. Just in case anyone was wondering, which they most assuredly were not.

[SimenB]

If you need a fresh environment for every single test, just put them in individual test files. The jsdom env is recreated for each test file. I don't think an API for this makes much sense, as it's quite edge casey

[m0rjc]

I'm testing code based on Window.postMessage() at the moment, using event listeners to look for messages coming from the component under test. or instantiating it and having it listen.

I've managed to solve a fair amount by creating an iframe, using its contentWindow, then throwing that away. But I'm still using the global window object in some tests.

[kohlmannj]

We're writing some tests that inject <script> tags into the DOM, but only if a certain window global doesn't already exist. It'd help us to have some kind of reset method for the JSDOM window instance so that we don't have to worry about performing cleanup for each specific window global.

[BlackGlory]

Without this feature, it seems impossible to reset properties like window.history.

[jhildenbiddle]

Slow and working is better than fast and broken.

Jest should offer the ability to reset jsdom on each test and let users determine if the extra 1/10th of a second performance hit per reset is tolerable for them. Examples have been provided here that clearly demonstrate why this is necessary and how simple resets like document.body.innerHTML = "" are insufficient.

Optimizing for performance is fine, but neglecting to offer a "slower but safer" path simply because it adds a minuscule amount of time per test is hostile to end users and feels weirdly (and unnecessarily) performance obsessed.

[jhildenbiddle]

For those interested, here is the soft-reset I'm using in jest.setup-tests.js which does the following:

• Removes event listeners added to document and window during tests
• Removes keys added to document and window object during tests
• Remove attributes on <html> element
• Removes all DOM elements
• Resets document.documentElement HTML to <head></head><body></body>

const sideEffects = {
  document: {
    addEventListener: {
      fn: document.addEventListener,
      refs: [],
    },
    keys: Object.keys(document),
  },
  window: {
    addEventListener: {
      fn: window.addEventListener,
      refs: [],
    },
    keys: Object.keys(window),
  },
};

// Lifecycle Hooks
// -----------------------------------------------------------------------------
beforeAll(async () => {
  // Spy addEventListener
  ['document', 'window'].forEach(obj => {
    const fn = sideEffects[obj].addEventListener.fn;
    const refs = sideEffects[obj].addEventListener.refs;

    function addEventListenerSpy(type, listener, options) {
      // Store listener reference so it can be removed during reset
      refs.push({ type, listener, options });
      // Call original window.addEventListener
      fn(type, listener, options);
    }

    // Add to default key array to prevent removal during reset
    sideEffects[obj].keys.push('addEventListener');

    // Replace addEventListener with mock
    global[obj].addEventListener = addEventListenerSpy;
  });
});

// Reset JSDOM. This attempts to remove side effects from tests, however it does
// not reset all changes made to globals like the window and document
// objects. Tests requiring a full JSDOM reset should be stored in separate
// files, which is only way to do a complete JSDOM reset with Jest.
beforeEach(async () => {
  const rootElm = document.documentElement;

  // Remove attributes on root element
  [...rootElm.attributes].forEach(attr => rootElm.removeAttribute(attr.name));

  // Remove elements (faster than setting innerHTML)
  while (rootElm.firstChild) {
    rootElm.removeChild(rootElm.firstChild);
  }

  // Remove global listeners and keys
  ['document', 'window'].forEach(obj => {
    const refs = sideEffects[obj].addEventListener.refs;

    // Listeners
    while (refs.length) {
      const { type, listener, options } = refs.pop();
      global[obj].removeEventListener(type, listener, options);
    }

    // Keys
    Object.keys(global[obj])
      .filter(key => !sideEffects[obj].keys.includes(key))
      .forEach(key => {
        delete global[obj][key];
      });
  });

  // Restore base elements
  rootElm.innerHTML = '<head></head><body></body>';
});

This has worked well for me so far. Not as well as a native Jest reset likely would, but... you know. :wink:

[trusktr]

Nice @jhildenbiddle.

Also, it is easy to import jsdom and just create your own context in your tests, regardless if you're even using Jest or not.

For example, you can make a function like this:

const { JSDOM } = require('jsdom');

function initJSDOM(markup, options = {}) {
  const dom = new JSDOM(markup, options);

  global.window = dom.window;
  global.document = dom.window.document;
  global.navigator = dom.window.navigator;
  global.location = dom.window.location;
  global.XMLHttpRequest = dom.window.XMLHttpRequest;

  return dom;
}

then use initDOM in any test to make a brand new JSDom context.

[elias6]

@jhildenbiddle overall your idea looks impressive. But I have a question. Is it OK to set the innerHTML property of document.documentElement to something with an html tag? I looked at document.documentElement.innerHTML on some random pages and it seems to only include the stuff inside the html tag, not the html tag itself.

The obvious solution would be to assign to document.documentElement.outerHTML instead, but that doesn't work when I try it. I get errors saying NoModificationAllowedError: Modifications are not allowed for this document. I can't think of anything better.

[jhildenbiddle]

Good catch, @elias6.

The following line:

  rootElm.innerHTML = '<html><head></head><body></body></html>';

Should be:

  rootElm.innerHTML = '<head></head><body></body>';

I've updated the code in the comment above.

FYI, here's the explanation for why you were unable to set document.documentElement.outerHTML from MDN:

If the element has no parent element, setting its outerHTML property will not change it or its descendants. Many browsers will also throw an exception.

Since document.documentElement is the <html> element, it has no parent node and therefore throws an error.

[elias6]

@jhildenbiddle what if there are attributes on the html element? How can those be cleaned up? I am doing something like this in my tests:

[...document.documentElement.attributes].forEach(attr =>
    document.documentElement.removeAttribute(attr.name)
);

But I am wondering if that catches everything, or if there is a somehow better way to do what I am doing.

[jhildenbiddle]

@elias6 --

I'm doing the same thing in the beforeEach block of the reset:

const rootElm = document.documentElement;

// Remove attributes on root element
[...rootElm.attributes].forEach(attr => rootElm.removeAttribute(attr.name));

No reason to think it wouldn't remove all of the attributes since that's exactly what removeAttribute() is supposed to do. :)

[elias6]

@jhildenbiddle I know that my idea removes all the attributes on the html element. I was asking about whether there is any other important state that it fails to remove. But I'm guessing the code in your comment handles that.