Fix uplink and search issues. #5357
Conversation
… to put the storage in a separate volume.
…odified time of the package manifest.
…nse. Respect the base url of the uplink when performing a search.
|
Hi Daniel, Thank you for the contributions! There are some good finds in there. It shows that you spent a lot of time using and testing the search and uplink feature (and we didn't), and studying the repo to come up with fixes. Thanks! Based on your GitHub history, it appears that you are new to open-source development. I was there some years ago as well. Let me help you see your pull request from our maintainer's perspective. Several changes in this one are unrelated to each other. This makes it very hard to review what changed and difficult to test if it works. For example, let's say we disagree with the Dockerfile change. We can't merge this pull request, but that negates everything else you did. Or we end up with a long back-and-forth discussion of what's good and what's not. No one has time for that. Keep your PRs small. The 17 commits should have been at least 10 pull requests (maybe even 17 ;-) ). Many of them we could approve and merge right away (with some tests). There's a lack of tests. This project contains many tests, which makes it a great open-source solution. Your changes pass all tests, which is good. However, we don't have tests that cover your fixes. Ideally, it should be tests that fail with the current code, but pass with your fix. Not everything needs a test, but important changes, for example, when you say "it crashes," certainly must have one. See updates. It helps to open issues before starting to code. We might agree that there is a bug, but not how it was fixed. That wastes your time. It's better to create an issue, document what's not working, and discuss how to approach a fix. For example, had you posted the list above as an issue, we could have avoided this big pull request. It also helps to add examples. You wrote "this simplifies configuration," but how? If you post your configuration as an issue, we could easily follow your thinking and provide feedback before we jump to code. Finally, a couple of technical things. The repo is using changesets. For each pull request, you need to add one. See contribution guidelines. For the title of pull requests, we follow conventional commits. For most of your changes, that would be a "fix:" prefix in the title. Keep up the good work! I will let @juanpicado chime in on how to move forward... |
This commit series addresses the following issues:
1. URL of uplink is not respected when performing a search.
2. Incorrect uplink is chosen when downloading a tarball which can result in an authentication failure and a crash.
3. The /verdaccio/storage volume is required, not an optional requirement of Docker deployments.
4. Manifest file is only retrieved from the first uplink containing a given package which prevents versions from being spread across multiple uplinks.
5. NPM search crashes when maintainer email is not provided.
6. Total search results does not match the actual number of packages available.
7. Package search results is missing license information.
8. Package search results is missing publisher information.
9. Search is aborted on connection close instead of on a connection error.
10. npm link is specified in the search results even though it's not required.
11. scope is specified in the search results even though it's not required.
With the above changes, it is now possible to specify private uplinks in the catch-all and scoped packages configurations before (recommended) or after the npmjs proxy. This greatly simplifies the configuration for most use-cases.