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

Drops unnecessary dependency on yarn in favor of npm, always available. #33

Closed
wants to merge 1 commit into from

Conversation

Satyam
Copy link
Contributor

@Satyam Satyam commented Apr 4, 2018

To make wider distribution easier, once downloaded, it is good manners that a package should not fail on the npm i command.

Making implicit dependencies on globally installed packages (thus not present in the dependencies lists of package.json) is not a good idea, specially when there is no real need.

Partially fixes #30

@gioragutt
Copy link
Contributor

I don't understand your fix. The fix should be adding the implicit dependencies as devDependencies, but you're not doing that.

As of this instance, this works with yarn, and not with npm, so unless you fix this precise issue, this PR can't be merged.

@Satyam
Copy link
Contributor Author

Satyam commented Apr 6, 2018

Adding Yarn to the devDependencies does not work 100% because while the installation would probably work (I haven't tested it myself) npm i would have ignored the yarn.lock file resulting in a potentially different install. The same applies to my solution.

Contrary to what its name indicates the prepublish script is run both before publishing and after installing which has caused so much confusion that it is being deprecated.

However, while Npm users will probably not be aware of the lock-file issue and this dismiss it, Yarn users probably would and, if any issue arises on installation, they might easily fix it by falling back to Npm since everyone has Npm while non-Yarn users would simply be stuck.

The full solution would be to provide both yarn and npm lock files as suggested in this comment within a long standing conversation about this issue.

Anyway, in-house usage and world-wide adoption require two different criteria and, whatever your internal preferences, Npm is best for the wider audience. None of this affects library users, just contributors.

@Satyam Satyam closed this by deleting the head repository Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Development section in README
2 participants