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

jest rootDir #23

Open
manuphatak opened this issue Feb 7, 2018 · 3 comments
Open

jest rootDir #23

manuphatak opened this issue Feb 7, 2018 · 3 comments

Comments

@manuphatak
Copy link

Hello 👋

At ThinkCERCA we're experiencing an issue with jest's config. Jest has a optimization tool called jest-haste-map which appears to walk the entire rootDir looking for javascript code. This includes node_modules from old frontend code in a different directory, it includes javascript included with ruby in our vendor directory. And on our CI server, it includes everything cached by npm & yarn in thinkCERCA/.semaphore-cache. As you might imagine, there's all sorts of clashes happening. For whatever reason, certain snapshot tests reliably fail on our CI server.

I found 2 solutions:

  1. Delete npm/yarn cache on our CI server.

  2. Change jest's rootDir from thinkCERCA/ to thinkCERCA/client

Since create-react-app limits our ability to customize our configuration, solution 2 required a little bit of hackery that's worth sharing.

I copied in @zeal/react-scripts/scripts/test.js and @zeal/react-scripts/scripts/utils/createJestConfig.js to our codebase and made some changes to move jest's rootDir. I posted a diff of the changes here: https://gist.github.com/bionikspoon/af66c22122dc19f57ebf0e04b0137935

About half the changes are a product of copying the file over to our local project directory, the other half are for moving jest's rootDir.

These changes may or may not belong in your repo, you decide. If they are, happy to help any way I can.

@randycoulman
Copy link

@bionikspoon Thanks for reporting this. It's an interesting problem, for sure.

TLDR: It might be worth opening this issue upstream on facebook/create-react-app. They may not want to fix it there, but either way, there's some work being done upstream that's going to have an impact on a solution for this problem, so we're going to wait until that work is available to us to see if that resolves the issue. We'll keep this issue open to remind ourselves to come back and check once we move to CRA 2.0.

Longer version:

It seems like this is an issue with the upstream CRA itself. I suspect that if you used the original create-react-app in your repo, it would have the same problems as this fork. I think it would be worth opening an issue on that repository to see what they say about it.

However, I think that CRA expects to own the entire repository where the app is generated, so they may deem this as something they don't want to fix. I might be wrong about that, but I think that would be a reasonable response on their part.

Since this fork is intended to be embedded within a parent app, it may fall to us to address this issue. The diff you provided seems reasonable in that light, with one minor concern:

What information does jest/jest-haste-map get from having the base project directory as its <rootDir> that it wouldn't get by narrowing <rootDir> down to just the client directory (or src directory in the upstream project)? That is, are we subtly breaking something by making this change?

In digging into this problem, I noticed that there's a bunch of work being done upstream to support lerna/yarn workspaces, and that work will have an impact on what our solution might look like. That work might even fix this issue as a side-effect. See facebook#3741 for example.

Because of that, we're going to hold off on addressing the issue here for now.

We'll keep this issue open and revisit after we can upgrade to CRA 2.0.

@bmatto
Copy link

bmatto commented Feb 9, 2018

@randycoulman Thank you for digging into this, very much appreciate the insights. As @bionikspoon mentioned the issues related to this are preventing snapshot testing 📸in our app. To that end our solution in the interim is going to be to pull these test scripts in locally to our project as seen in the diff provided.

Again, super appreciative of the time spent looking at this. We'll keep on eye for CRA 2.0 as I'm sure you all will as well.

@manuphatak
Copy link
Author

I second everything @bmatto said.

Thank you @randycoulman for looking into this so quickly and thoroughly. It is appreciated! 😄

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

3 participants