Skip to content

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

Merged
fson merged 1 commit into
react:masterfrom
fson:fix-mocking-assets-with-special-chars
Sep 10, 2016
Merged

Fix mocking assets with special characters in the file path#584
fson merged 1 commit into
react:masterfrom
fson:fix-mocking-assets-with-special-chars

Conversation

@fson

@fsonfson commented Sep 5, 2016

Copy link
Copy Markdown
Contributor

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.
@ghostghost added the CLA Signed label Sep 5, 2016
@gaearongaearon added this to the 0.4.2 milestone Sep 5, 2016
@ghostghost added the CLA Signed label Sep 5, 2016
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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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

vjeux commented Sep 6, 2016

Copy link
Copy Markdown
Contributor

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

@fson

fson commented Sep 10, 2016

Copy link
Copy Markdown
ContributorAuthor

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

@ghostghost added the CLA Signed label Sep 10, 2016
@gaearon

Copy link
Copy Markdown
Contributor

@fson 👍

@fson
fson merged commit cdd736d into react:masterSep 10, 2016
@fson
fson deleted the fix-mocking-assets-with-special-chars branch September 10, 2016 16:56
@vjeux

Copy link
Copy Markdown
Contributor

Yeah go for it :)

@gaearongaearon mentioned this pull request Sep 18, 2016
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 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.
@locklockBot locked and limited conversation to collaborators Jan 22, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@fson@vjeux@gaearon