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

Prettier is used by toMatchInlineSnapshot, but it is not a dependency of jest #8467

Closed
suchipi opened this issue May 14, 2019 · 16 comments
Closed

Comments

@suchipi
Copy link
Contributor

suchipi commented May 14, 2019

🐛 Bug Report

Prettier is used by jest for toMatchInlineSnapshot, but since it's not a dependency of jest, toMatchInlineSnapshot only works if some other package depends on prettier (or it's in the user's package.json).

To Reproduce

Steps to reproduce the behavior:

  • Make a new directory and run yarn init --yes inside it
  • Run yarn add jest
  • Create a test using the toMatchInlineSnapshot matcher, eg:
    expect(2 + 2).toMatchInlineSnapshot()
    
  • Notice that the test fails with an error indicating that prettier cannot be found.

Expected behavior

Test passes with no issues

npx envinfo --preset jest

  System:
    OS: macOS 10.14.4
    CPU: x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
  Binaries:
    Node: 12.1.0 - /usr/local/bin/node
    Yarn: 1.13.0 - /usr/local/bin/yarn
    npm: 6.9.0 - /usr/local/bin/npm
  npmPackages:
    jest: 24.8.0 => 24.8.0
@SimenB
Copy link
Member

SimenB commented May 14, 2019

This is expected. @azz might remember the particulars

@SimenB
Copy link
Member

SimenB commented May 14, 2019

Would you expect jest to bundle some version and defer to a local one? Related #7792

@suchipi
Copy link
Contributor Author

suchipi commented May 14, 2019

Hmm. Not sure; thanks for the context.

Maybe it should be listed as a peerdep? A descriptive error message saying "you need to install prettier" might be good enough, too. It's unexpected to get a "could not find prettier" error the first time you use .toMatchInlineSnapshot.

Not sure if this makes sense for jest, but prettier could be replaced with babel + recast here, and it could use prettier for printing (instead of recast) as an optimization, when it's available.

@SimenB
Copy link
Member

SimenB commented May 14, 2019

There's a bunch of discussion in the original PR #6380, especially #6380 (comment).

The main thing we wanna avoid is to completely butcher the styling of the test file creating churn. Using prettier assures we're consistent with the current styling, which is nice. Not sure how well using recast would work? It might work great without butchering existent styling at all for all I know 🙂

@suchipi
Copy link
Contributor Author

suchipi commented May 14, 2019

Yup, that's what it's designed for 👍 recast keeps track of which AST nodes have been changed and only prints those ones using original styling. It uses the original source for all the other nodes. You can defer its parsing to another parser, so babel could be used to ensure all the necessary syntax and etc is supported.

@SimenB
Copy link
Member

SimenB commented May 15, 2019

Oh, nice!

A descriptive error message saying "you need to install prettier" might be good enough, too

yeah, we should definitely to that as a first step regardless. #7744

@kevinbarabash
Copy link

I just ran into this as well.

@selfrefactor
Copy link

Still no progress?

@UziTech
Copy link

UziTech commented Apr 21, 2020

Maybe the error message should at least state that prettier must be installed to use toMatchInlineSnapshot. I just spent way too long wondering what was throwing the error. I definitely did not think jest was using a package without requiring it as a dependency. That seems like an obvious code smell.

@UziTech
Copy link

UziTech commented Apr 21, 2020

looks like it is supposed to throw a descriptive error message:

https://github.com/facebook/jest/blob/fa4cbbf87cb3c97fe44f1fee30663715a9dbba71/packages/jest-snapshot/src/inline_snapshots.ts#L33-L35

The error message that I get with [email protected] is:

Cannot find module 'prettier' from 'setup_jest_globals.js'

@jhunterkohler
Copy link
Contributor

jhunterkohler commented May 28, 2021

I am still getting this error:

    Cannot find module 'prettier' from 'node_modules/jest-jasmine2/build/setup_jest_globals.js'

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)

Not sure why it has stopped working, because I did not uninstall prettier, yet this no longer works. Maybe a sub-dependency did, not-sure.

Edit: Crossed out test, error has been resolved in Jest v27 (#11400)

@UziTech
Copy link

UziTech commented May 28, 2021

@HunterKohler This should be fixed in Jest v27 (#11400) What version are you using?

@jhunterkohler
Copy link
Contributor

@UziTech My bad! It is fixed, just had to update dependencies. Thank you.

@ali-target
Copy link

This issue hasn't been fixed, I still get an error.

"jest-cli": "^27.0.3",
"prettier": "^1.19.1",
"jest": "^27.0.3",

@SimenB
Copy link
Member

SimenB commented Feb 23, 2022

I think this is fine now that prettier is optional, as mentioned a few times above. If people are still having issues please create a new issue.

@SimenB SimenB closed this as completed Feb 23, 2022
@github-actions
Copy link

This issue 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 Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants