-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Build Tools: Validate package-lock.json for "resolved" errors #22237
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Nice verification, I verified the error triggers when expected.
Unfortunately it seems on travis we get "bin/validate-package-lock.js(19,30): error TS2307: Cannot find module '../package-lock'.", so the json file is not read as expected. |
@jorgefilipecosta Yeah, funny enough, quite similar to very recent discussion with @sirreal at #19866 (comment) . I think a |
On the wp-scripts package we read JSON files with:
Maybe using this approach works on travis? |
Size Change: +89 B (0%) Total Size: 824 kB
ℹ️ View Unchanged
|
Travis is green I guess we can merge :) |
🎉 |
Nice one, thanks for working on it 👍 |
Related Slack discussion (link requires registration): https://wordpress.slack.com/archives/C02QB2JS7/p1588692036240600
Related upstream issue: npm/cli#1138
Previously:
This pull request seeks to add a pre-commit and continuous integration build step for validating the contents of
package-lock.json
. Notably, this is intended to try to capture issues where an invalidresolved
value becomes introduced inpackage-lock.json
. The scriptbin/validate-package-lock.js
will deeply check the contents of the lockfile for anyresolved: false
.The idea for this change proposal was inspired by @adamziel 's comment at #21873 (comment).
Testing Instructions:
Introduce
"resolved": false
in a dependency inpackage-lock.json
(manually) and attempt to commit the change. Observe an error.