create-react-app icon indicating copy to clipboard operation
create-react-app copied to clipboard

Fix writing an absolute path to modulePaths in Jest config when ejecting

Open mkarajohn opened this issue 3 years ago • 3 comments
trafficstars

Resolves #8965

Problem & Cause

As described in the linked issue, when a CRA project is ejected, the modulePaths entry of the jest config includes an array of absolute paths, if a baseUrl is defined in the tsconfig.json or jsconfig.json.

The array is created here exported as additionalModulePaths.

Since this exported array of additionalModulePaths is also used in the webpack.config.js here, where absolute paths are in fact needed, we cannot be directly modifying the way the additionalModulePaths array contents are created.

Solution

What this PR does in order to fix the issue, is to translate the absolute paths to relative ones in the createJestConfig.js file here, since adding absolute paths in the modulePaths is problematic, in cases such as CI pipelines, or simply someone else trying to run the tests on their own machine. Making them relative to the rootDir solves any issues.

Before: image

After: image

mkarajohn avatar Jul 19 '22 14:07 mkarajohn

Hi @mkarajohn!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Jul 19 '22 14:07 facebook-github-bot

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot avatar Jul 19 '22 16:07 facebook-github-bot

I realized that the original fix would not work for paths longer than 1 directory deep, e.g. foo/bar as it was simply using path.basename() which, in such cases, does not translate to the correct path.

So, createJestConfig.js had to have some way to know the original baseUrl path, in order to translate it relative to rootDir. As such, a little change was made in ec12ba1, to the output of getAdditionalModulePaths(). It now returns an object like

type additionalModulePaths = {
  resolved: [string];
  relative: [string];
}

additionalModulePaths.resolved is used in the webpack.config.js modules resolution, while additionalModulePaths.relative is used in createJestConfig.js. These are the only 2 places where additionalModulePaths is used at all.

mkarajohn avatar Jul 19 '22 19:07 mkarajohn