Fix for jest handling of static assets when running tests. #457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks @raythree! 💯 Guys, this look good to you? |
|
Added a few comments, the fix itself all appears to be valid but a few things related to clean up. |
|
Thanks guys! Merging. |
kwelch
added a commit
that referenced
this pull request
Aug 15, 2017
kwelch
reviewed
Aug 15, 2017
| @@ -112,10 +112,10 @@ | |||
| "url": "https://github.com/coryhouse/react-slingshot" | |||
| }, | |||
| "jest": { | |||
| "moduleNameMapper": { | |||
| "\\.(css|scss)$": "identity-obj-proxy", | |||
| "^.+\\.(gif|ttf|eot|svg|woff|woff2|ico)$": "<rootDir>/tools/fileMock.js" | |||
There was a problem hiding this comment.
Why not update the fileMock to have the changes or fully deprecate it.?
| } | ||
| "moduleNameMapper": { | ||
| "\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/tools/assetsTransformer.js", | ||
| "\\.(css|less)$": "<rootDir>/tools/assetsTransformer.js" |
There was a problem hiding this comment.
Any reason to switch from scss to less? Also, why not put this in the regex prior? It also may be simpler if the regex was anything that is not js|jsx like ^(jsx?)$
There was a problem hiding this comment.
No worries I made these changes directly to master. The not regex did not work as I had intended.
coryhouse
pushed a commit
that referenced
this pull request
Sep 3, 2017
* removed react-router form package.json & installed react-router-dom. * completed index.src refactor. * re-implemented App.js * added notes to import changes & App.js * modified middleware array @ store/configureStore.js * updated yarn.lock file with fresh `yarn` command. * updated master branch. * upgraded react-hot-reloader to @3.0.0-beta.6 * removed extra "document.getElementById(app) from Root.js * moved react-router-redux to regular "dependencies" from "dev-dependencies" * cleaned Root.propTypes. * cleaned Root.js & App.js per @oshalygin feedback. * replaced .gitignore comment. * removed react-router form package.json & installed react-router-dom. * completed index.src refactor. * re-implemented App.js * added notes to import changes & App.js * Update FAQ.md fixed header on FAQ page * Update README.md fixed a few headers improperly declared * modified middleware array @ store/configureStore.js * Add react hot loader 3 (#392) - This commit wires up react-hot-loader 3 to be used in the application. There are numerous benefits to the latest release, all of which can be seen at https://github.com/gaearon/react-hot-loader - Note that the specific implementation around wrapping in a Root component is part of how react-hot-loader 3 needs to be configured. - Note that the package is brought in as a dependency, not a dev dependency because of how it is switched at runtime or not. - More information on the migration can be viewed at: https://github.com/gaearon/react-hot-loader/tree/master/docs\#migration-to-30 Related: #216 * Updated react-hot-loader to correct package version. (#401) * Add item to check if issues * updated yarn.lock file with fresh `yarn` command. * Fix formatting (#403) * updated master branch. * upgraded react-hot-reloader to @3.0.0-beta.6 * removed extra "document.getElementById(app) from Root.js * moved react-router-redux to regular "dependencies" from "dev-dependencies" * cleaned Root.js & App.js per @oshalygin feedback. * replaced .gitignore comment. * clean index.js * Add CONTRIBUTE.md (#431) * Add * Updated yarn lock using upgrade * Rename * Update * Upgrade to webpack 3 * Update yarn lock * Update snapshot * Set prod env when analyzing bundle * Add jest-cli as dependency * Revert PR #450 (#451) Removed change that removed additional dashes in npm test scripts in favor of adding jest-cli as a devDep. This commit instead focusses on issue 2 from #449 where setupPrompts.js had a bug that caused start script to fail. * Issue #449 fix (#450) * Issue #449 fix Issue 1: Removed extra dashes located in the package.json test scripts that cause start script to fail. Issue 2: Removed escape characters found in setupPrompts.js which cause linting to fail, thus breaking start script. * Jest fix Re-added previously removed dashes from test scripts in package.json that caused start script to fail. Instead, @coryhouse added in jest-cli as a dev-dep which resolves the issue. * Enhance babel env config to transpile for IE9+ (#452) * Fix for jest handling of static assets when running tests. See: (#457) jestjs/jest#2663 (comment) * Added tips for npm run lint and build errors (#151) (#460) * pushing changes from upstream fetch. * updated from rebase. * modified package versions in package.json & created new build. * update package.json * fixed conflicts with upstream master. * cleaned up PropTypes validations - react-router-redux throwing PropTypes error. * comment spell check & de-console on Root.js
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for jest handling static assets
jestjs/jest#2663 (comment)