facebook/react

Problems with Mocha / Jsdom / React testing

Munter opened this issue · 9 comments

I'm not entirely sure if what I'm having here is a problem with Mocha or React, but I really need some help figuring out why my tests are failing with what looks like leaks across siloed documents.

My test setup is using Mocha, where each individual test spins up a document with jsdom before loading React. I then proceed to run the test of the individual component in each of these separate documents.

My tests run fine if I only run a single file at a time, but when I run all tests I get errors like DOMException: Wrong document (from jsdom) and Error: Invariant Violation: findComponentRoot(..., .1): Unable to find element... (from React).

I've created a repository with a minimal setup and a more detailed description here: https://github.com/Munter/MochaJsdomReact

Can anyone help me figure out what the problem is, and how to avoid it, or maybe even fix it?

zpao commented

If you clear your require cache does this start working? I wouldn't be surprised if some of our node memoization causes issues (namely in this case, the bit we use to generate HTML with the right namespace creates a node once). Since these are all run in the same process, the requires are cached. Looks like the feature was rejected from mocha so you may have to make it happen yourself or just run each on in its own process. mochajs/mocha#536

I have tried clearing the require cache for the specific tests that cause these problems. Doing so just ends up propagating the problem to some later test in the chain that previously didn't fail, but now suddenly does. The whole thing is further complicated by a requirejs indirection, making it really hard to debug in the non-minimal case.

Would it hurt React performance a great deal to not cache these nodes from the original document?

Any chance we could configure React to disable memoization for testing?

zpao commented

Yes, creating a new node each time would be expensive. Potentially we could check if the cached node's ownerDocument matches window.document. Even just doing that check will come at a cost though and I'm not sure it's worth it to support a use case we don't currently intend to support.

React isn't really configurable like that and it would be more of undertaking than we plan on making to do so. Maybe one day.

Any idea on why the error in this example doesn't happen without the setState-call and also not if the line of text is removed?

In my real life tests the place where it breaks is even more absurd, there's an image tag in a different component, if I remove that the tests run. Nothing special about that image. I'd really like to understand the underlying logic here

zpao commented

It only happens on re-renders (initial render just does an innerHTML), which is why setState triggers it.

As for the why, when we create new nodes to insert, we have a string of html, add it to a node, then extract the DOM node. Then we put it in the right place. We do this because some nodes need to be wrapped to insert correctly. Things probably work fine up until the point where we move the nodes from the dummy wrapper and put them into the right place. This is where we cross document elements (I think that's the "Wrong document" error).

I don't know what's happening in your image case specifically, could be related to an update. Creating nodes is the problem, updating existing nodes should be ok.

Thanks for the explanation. It does look like removing the React module from the require cache before each test does indeed alleviate that error. The test also run a bit slower, which is not surprising with the repeated disk reads I guess.

@Munter Can you give a bit more info on how you ended up getting this to work? I've tried putting this in a beforeEach() block in my test:

delete require.cache[require.resolve('react')];

But no dice. Everything works on first mocha test run, but if I'm using --watch and save any file, the test blows up with wrongDocumentErrors.

Using React 0.14.3.

@jasonblanchard We are running our tests with requirejs as a module loader. The cache clearing part is here: https://github.com/podio/jsdomify/blob/master/src/jsdomify.js#L38-L44

On our tests we are using the packaged version of React, which means that what you are doing will work. If you are running against the React source, then it's probably the specific files that create and cache DOM nodes that you explicitly have to clear from the require cache.

I'd really love if React exposed a method to clear out any cached nodes in a non-public api