search

riasvdv / laravel-mix-eslint

mix.eslint()
20 stars 5 forks source link

Advise against forcing `yarn` dependency installation #5

Closed mikeerickson closed 5 years ago

mikeerickson commented 5 years ago

First of all, thanks for the quick plugin.

However, I would like to suggest that you refrain from assuming the user is using yarn as their package manager. If you have missing dependency, I would suggest you mention it as an additional requirement in the docs.

When you take it upon yourself to automatically install the dependencies using yarn, if the project is using npm (as in my case and surely others) it forces a complete reinstall of all modules (which is time consuming) and then switches the project to assume yarn moving forward.

If the user then installs another module down the road using npm, it will again force a complete reinstall of packages (another long process) and then you have started a vicious cycle as your installation of eslint-loader does not save it into the package.json thus step is repeated unless developer is keen to notice what is happening.

IMHO, this is bad practice to make the assumption, and I would recommend to instead notify the user that required dependencies are not installed and leave it at that (you do indeed issue this notification).

mikeerickson commented 5 years ago

And FWIW, you are not the only offender of this. Vue does this as well itself with the vue-template-compiler package (perhaps this is where you drew your inspiration)

riasvdv commented 5 years ago

This plugin does nothing to enforce either yarn or npm, dependency management is handled by the laravel-mix package.

There is not even a single mention of yarn in this repository.

If you have issues with how laravel-mix handles things, make an issue there https://github.com/JeffreyWay/laravel-mix