Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Allow moduleNameMapper config override #303

Merged
merged 4 commits into from
May 9, 2018

Conversation

sebald
Copy link
Contributor

@sebald sebald commented Apr 11, 2018

This patch will allow path mapping to be fully supported, by allowing moduleNameMapper to be configured.

Issue

Currently, you can add paths to your tsconfig and the app will run fine. But jest / ts-jest does not use the configuration from your tsconfig. Configuring the moduleNameMapper will solve this. Sadly, the property is not "supported" and tests can not be run if the configuration is set.

Example usage:

Changing the following files will allow you to import modules from the src folder, by prefixing the import path with a tilde. E.g. a component living in src/components/Button can always be imported via ~/components/Button. No ../../../ anymore!

tsconfig.json

{
  "compilerOptions": {
     ...
+    "baseUrl": ".",
+    "paths": {
+      "~/*": ["src/*"]
+    }
  },

package.json

{
  ...
  "jest": {
    ...
+    "moduleNameMapper": {
+      "^~/(.*)$": "<rootDir>/src/$1"
+    }
  },
}

Changes:

  1. Add baseUrl so tsconfig-paths-webpack-plugin will not warn that it is missing (Fixed problem with tsconfig.json baseUrl and paths #223)
  2. Allow overriding moduleNameMapper (Support for TypeScripts 'paths' option #203)

Allow overriding the moduleNameMapper configuration is the pragmatic approach. If you're using absolute paths + react-native you would have to re-add the mapping.

@farzadmf
Copy link

farzadmf commented May 1, 2018

Any news on this? Any plan to merge this PR to have moduleNameMapper support?

@@ -75,6 +75,7 @@ module.exports = (resolve, rootDir, isEjecting) => {
'coverageReporters',
'coverageThreshold',
'snapshotSerializers',
'moduleNameMapper',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This should be listed in the corresponding README to be adjustable. Just recognized that the original section on CRA (https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#configuration) is absent here... However, this field (and potentially the section) should be listed there, including the example usage and a note on how this relates to entries in the tsconfig.json.
  • I'm still thinking about the default value used on this field, '^react-native$': 'react-native-web'. It should either be noted that it has to be provided manually in case of overriding this key, or that entry should be merged automatically. Suppose the first way might be preferable.

@DorianGrey
Copy link
Collaborator

I'm very sorry for the late response, I was quite busy lately and also attempted to figure out why the travis build began to fail shortly after it was fixed for the master branch ...

Comments are attached on the review.

@sebald
Copy link
Contributor Author

sebald commented May 6, 2018

@DorianGrey No problem, I'll integrate the suggested changes next week 🙂

Sebastian Sebald added 3 commits May 8, 2018 11:15
* master:
  Stick @types/node to 9.6.7 to avoid problems with 10.0.0 (wmonk#315)
  Fix travis build (wmonk#302)
  v2.15.1
  Update README For 2.15.1
  fix(modules): remove duplicate mjs from jest config
@sebald
Copy link
Contributor Author

sebald commented May 8, 2018

@DorianGrey added the section to the readme as suggested 🙂

@DorianGrey DorianGrey merged commit e17362e into wmonk:master May 9, 2018
@DorianGrey
Copy link
Collaborator

LGTM, thanks!

(Note: I'll ignore the failing CI for the moment - that kitchensink case is extremely unstable, still working on a fix for that).

@sebald sebald deleted the allow-moduleNameMapper-override branch May 9, 2018 06:58
@adambowles
Copy link
Contributor

Nice! When will this be released to npm?

@DorianGrey
Copy link
Collaborator

I'm still waiting for a reaction on #314 to figure out if pinning the @types/node version can be reverted or has to be retained. Would be favorable to figure this out before an additional release.

DorianGrey added a commit that referenced this pull request May 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants