Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Check package-lock.json / yarn.lock / node_modules to enable prettier #191

Open
kachkaev opened this issue Jun 10, 2017 · 4 comments
Open

Comments

@kachkaev
Copy link
Member

Following this tweet: https://twitter.com/RobAWise/status/873443627036549120

Hi guys! Great idea to add this option!

Just curious if the package could also check the contents of package-lock.json / yarn.lock / node_modules in addition to package.json This can be useful for ‘monolithic’ deps similar to react-scripts (create-react-app).

In our team we have an internal linting npm module, which includes eslint-config, eslint itself and prettier-eslint-cli. The two latter packages are not explicitly mentioned in package.json, but node_modules/.bin/eslint and node_modules/.bin/prettier-eslint-cli do exist. This means that we can lint via npm scripts, but can't use the new tick in prettier-atom yet. linter-eslint does seem to detect eslint from node_modules, because linting as-you-type activates correctly in projects when eslint is a dependency of a dependency.

How easy would it be to make the check a bit smarter?

Cheers! Awesome plugin!

@olsonpm
Copy link
Contributor

olsonpm commented Feb 10, 2018

Wouldn't your solution mean that any package installed in the entire dependency tree which happens to depend on prettier will cause this flag to trip? It just seems like it would cause more unexpected behavior than otherwise.

I'd have to know more about your app structure, but it seems your internal linting module should declare 'prettier' as a peer-dependency given your consuming projects are expected to use prettier formatting directly. If you wanted to keep your setup as-is, I would expect your internal linting module to need its own atom plugin.

@kachkaev
Copy link
Member Author

The apps I’m referring to use @our-team/lint as a dev dependency. This module contains eslint, prettier as well as custom rules. The idea is that it’s installed in one go for any new project - it is only necessary to add a couple of scripts to package.json after npm install -D @our-team/lint.

I’m not sure that the side-effect scenarios will occur very often and I also guess that they will likely an issue in the module being included. That is because prettier needs to be a dev-dependency unless there are plans to use it like in my case.

@olsonpm
Copy link
Contributor

olsonpm commented Feb 10, 2018

but your team's linter dependency still expects consumers to be formatted by prettier, thus i still think it should declare prettier as a peer dependency.

and I'd personally be less concerned about how often the errors occur and more concerned with how unexpected the behavior would be when it does happen. I'm just one opinion though.

@kachkaev
Copy link
Member Author

kachkaev commented Feb 10, 2018

I'm not very strong in my opinion. However, given there are a few 👍 , others seem to be in the same situation too.

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

3 participants