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

added npm-shrinkwrap #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added npm-shrinkwrap #27

wants to merge 1 commit into from

Conversation

jehy
Copy link

@jehy jehy commented Jan 30, 2020

See #26

Added shrinkwrap file - with it, you can just run npm install and npm test. Also, transitive dependencies will be respected and same fails as on tests won't happen on production.

The only drawback is that it user will have many dependencies. This is a hack which should be avoided but it is better that having code that works wrong.

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I’m very against this; only apps should have lockfiles, this is a published package.

@jehy
Copy link
Author

jehy commented Jan 30, 2020

Yup. But right now deps are broken. We need either to fix them or freeze them. And fixing will be really hard - and I suppose impossible if you want to keep compatibility with node 4.

@ljharb
Copy link
Collaborator

ljharb commented Jan 30, 2020

Hard or not, fixing them without breaking compat is the right thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants