Skip to content

fix(srcclr): Update package-lock.json to satisfy SourceClear #215

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

Merged
merged 2 commits into from
Feb 4, 2019

Conversation

nchilada
Copy link
Contributor

@nchilada nchilada commented Feb 2, 2019

Summary

SourceClear seems to install lodash@4.17.10 per our package-lock.json, rather than installing lodash@4.17.11 (the latest) as they might if installing directly via npm. Update package-lock.json to mitigate this.

Preserve "lodash": "^4.0.0" in package.json so that our customers remain free to control their lodash subdependencies.

Issues

  • OASIS-4112

@nchilada
Copy link
Contributor Author

nchilada commented Feb 2, 2019

@mikeng13 @spencerwilson-optimizely @mjc1283 does this seem reasonable?

Or should we specify a more recent version of lodash in our package.json? That would make it easier for our customers to install the latest (and presumably safer) subdependency, with the caveats that this would marginally bloat their node_modules/ and would not really make sense unless we were to continually re-release our package with latest dependencies.

@coveralls
Copy link

coveralls commented Feb 2, 2019

Coverage Status

Coverage remained the same at 97.468% when pulling 3caa5c0 on nikhil/lodash into dcead1e on master.

@nchilada
Copy link
Contributor Author

nchilada commented Feb 3, 2019

I've confirmed that this works

Copy link
Contributor

@spencerwilson-optimizely spencerwilson-optimizely left a comment

Choose a reason for hiding this comment

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

Your understanding of how things work sounds totally accurate to me. This change only affects places that install deps based on package-lock.json--i.e., where people run npm install with local clone of the package source (e.g., local dev machines, CI).

@nchilada
Copy link
Contributor Author

nchilada commented Feb 4, 2019

Thanks @spencerwilson-optimizely! Yeah, the remaining question is whether we should be doing more and ensuring that we always use a secure version of lodash when installed as a dependency. Presumably we haven't cared in the past, given that we only specify ^4.0.0.

The challenge, of course, is that we'd have to keep re-releasing if our philosophy is to pin to secure packages.

That's a gnarly problem so I'm just going to merge this change for now!

@nchilada nchilada merged commit b8791d4 into master Feb 4, 2019
@nchilada nchilada deleted the nikhil/lodash branch February 4, 2019 18:48
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.

3 participants