Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with Mocha / Jsdom / React testing #4025

Closed
Munter opened this issue Jun 4, 2015 · 9 comments
Closed

Problems with Mocha / Jsdom / React testing #4025

Munter opened this issue Jun 4, 2015 · 9 comments

Comments

@Munter
Copy link

Munter commented Jun 4, 2015

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
Copy link
Member

zpao commented Jun 4, 2015

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

@zpao zpao closed this as completed Jun 4, 2015
@Munter
Copy link
Author

Munter commented Jun 4, 2015

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?

@dmatteo
Copy link
Contributor

dmatteo commented Jun 4, 2015

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

@zpao
Copy link
Member

zpao commented Jun 4, 2015

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.

@Munter
Copy link
Author

Munter commented Jun 4, 2015

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
Copy link
Member

zpao commented Jun 4, 2015

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.

@Munter
Copy link
Author

Munter commented Jun 5, 2015

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.

@jasonblanchard
Copy link

@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.

@Munter
Copy link
Author

Munter commented Dec 21, 2015

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants