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

[jest-config] Changed "Preset ... not found" validation error #8924

Merged

Conversation

EvanBacon
Copy link
Contributor

Summary

When using a jest preset that contains a general Error (not a SyntaxError or TypeError) jest-config throws a "Preset ... not found" validation error.
This is somewhat misleading for users of a broken preset that may just be missing a peer dependency.

Work-around

To get around this in jest-expo I wrap the requires of peer dependencies in try/catch, rethrow the meaningful error message, then exit the process to prevent users from thinking the preset is missing.

Test plan

  • Attempting to require any missing library in a Jest preset should result in a useful error message like:
● Validation Error:

  Error resolving jest-expo/web:

  Cannot find module 'react-native-web'
  Error: Cannot find module 'react-native-web'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:690:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at getBaseWebPreset (/Users/evanbacon/Documents/GitHub/expo/packages/jest-expo/src/getPlatformPreset.js:50:3)
    at getWebPreset (/Users/evanbacon/Documents/GitHub/expo/packages/jest-expo/src/getPlatformPreset.js:67:10)
    at Object.<anonymous> (/Users/evanbacon/Documents/GitHub/expo/packages/jest-expo/web/jest-preset.js:3:18)
    at Module._compile (internal/modules/cjs/loader.js:776:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)

  Configuration Documentation:
  https://jestjs.io/docs/configuration.html

As opposed to the current error message:

● Validation Error:

  Preset jest-expo/web not found.

  Configuration Documentation:
  https://jestjs.io/docs/configuration.html
  • I'm willing to write a unit-test but would like to get validation on this approach first.

When using a jest preset that contains a general error (require('library-that-probably-does-not-exist')) the misleading "Preset ... not found" error is thrown, even though the preset is found.
@EvanBacon
Copy link
Contributor Author

The CI TS check appears to be broken

@SimenB
Copy link
Member

SimenB commented Sep 7, 2019

CI failure is a lint error - the the TS warning is just a version mismatch warning (waiting for typescript-eslint/typescript-eslint#916)

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Barring lint errors this LGTM. Thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #8924 into master will decrease coverage by <.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8924      +/-   ##
==========================================
- Coverage   64.29%   64.28%   -0.01%     
==========================================
  Files         276      276              
  Lines       11707    11711       +4     
  Branches     2864     2865       +1     
==========================================
+ Hits         7527     7529       +2     
- Misses       3549     3550       +1     
- Partials      631      632       +1
Impacted Files Coverage Δ
packages/jest-config/src/normalize.ts 80.93% <71.42%> (-0.4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 116303b...8283889. Read the comment docs.

@SimenB SimenB merged commit 0df9981 into jestjs:master Sep 7, 2019
@EvanBacon EvanBacon deleted the @evanbacon/jest-config/update-errors branch September 7, 2019 05:52
aleclarson pushed a commit to aleclarson/jest that referenced this pull request Nov 15, 2019
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
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.

4 participants