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

Add linter for package-lock.json resolved #30719

Closed
wants to merge 2 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Apr 26, 2024

If the user is using a npm proxy, the will write its URL into package-lock.json, like it happened in #28571, which may cause issues in the future in case npm ever decides to use of these URLs (currently it doesn't seem to).

Add this linter to ensure all URLs in the files start with https://registry.npmjs.org/.

Example error:

package-lock.json: Expected "resolved" on package node_modules/@aashutoshrathi/word-wrap to start with "https://registry.npmjs.org/"

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 26, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 26, 2024
@silverwind
Copy link
Member Author

silverwind commented Apr 26, 2024

Actually it seems it's really not a problem because npm and likely other package managers ignore the resolved property. It seems to serve no purpose and is likely a artifact from past npm versions that used it, but as per https://stackoverflow.com/questions/53814967 npm does not use it any more since v5.

Edit: Not sure if it holds correct but as per npm/npm#16849 (comment) it does use it for some form of cache that invalidates when the url does not match the current registry url, so still, harmless.

@silverwind silverwind closed this Apr 26, 2024
@silverwind silverwind deleted the lint-misc branch April 26, 2024 14:18
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/internal modifies/js size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants