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

Fix mocking assets with special characters in the file path #584

Merged
merged 1 commit into from
Sep 10, 2016

Conversation

fson
Copy link
Contributor

@fson fson commented Sep 5, 2016

The regexes in the Jest moduleNameMapper configs were a bit too strict,
causing them to not pick up files with special characters like @ in the
file path. Change them to match anything with the correct file extension.

Fixes #579.

Test plan:

  • Created a file named template/src/logo@2x.png.
  • Added import './logo@2x.png'; to App.js.
  • Ran npm test. Tests pass.

The regexes in the Jest `moduleNameMapper` configs were a bit too strict,
causing them to not pick up files with special characters like `@` in the
file path. Change them to match anything with the correct file extension.
@ghost ghost added the CLA Signed label Sep 5, 2016
@gaearon gaearon added this to the 0.4.2 milestone Sep 5, 2016
@ghost ghost added the CLA Signed label Sep 5, 2016
@@ -21,8 +21,8 @@ module.exports = (resolve, rootDir) => {
const config = {
moduleFileExtensions: ['jsx', 'js', 'json'],
moduleNameMapper: {
'^[./a-zA-Z0-9$_-]+\\.(jpg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm)$': resolve('config/jest/FileStub.js'),
'^[./a-zA-Z0-9$_-]+\\.css$': resolve('config/jest/CSSStub.js')
'^.+\\.(jpg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm)$': resolve('config/jest/FileStub.js'),
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are going to ignore everything before the end, you can just omit ^.+ altogether and just write \\.css$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea. However, turns out the files don't get stubbed, if I change this to \\.css$. This looks like a possible bug in Jest to me (unless it's only supposed to work with regexes that match the whole path). When given a non-anchored regex, this line doesn't replace the module name correctly.

I suggest that we merge this PR as-is, with the anchored regex, and we can open an issue in Jest about regexes like \\.css$ not working correctly.

@vjeux
Copy link
Contributor

vjeux commented Sep 6, 2016

Accepted. Feel free to merge it once you apply my comments :)

@fson
Copy link
Contributor Author

fson commented Sep 10, 2016

@vjeux Mind if I merge this without your suggestion? Jest doesn't work with the partial regex at the moment.

@ghost ghost added the CLA Signed label Sep 10, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 10, 2016

@fson 👍

@fson fson merged commit cdd736d into facebook:master Sep 10, 2016
@fson fson deleted the fix-mocking-assets-with-special-chars branch September 10, 2016 16:56
@vjeux
Copy link
Contributor

vjeux commented Sep 10, 2016

Yeah go for it :)

@gaearon gaearon mentioned this pull request Sep 18, 2016
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
…#584)

The regexes in the Jest `moduleNameMapper` configs were a bit too strict,
causing them to not pick up files with special characters like `@` in the
file path. Change them to match anything with the correct file extension.
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants