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

artifact urls for scoped package format doesn't play nicely with yarn --offline #1310

Closed
moon-stripe opened this issue May 13, 2019 · 12 comments

Comments

@moon-stripe
Copy link

Describe the bug
A clear and concise description of what the bug is.

verdaccio returns artifact urls for scoped packages that look like this:
https://{verdaccio-url}/@babel%2fcode-frame/-/code-frame-7.0.0.tgz#06e2ab19bdb535385559aabb5ba59729482800f8

as opposed to in public registry:

https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.0.0.tgz#06e2ab19bdb535385559aabb5ba59729482800f8

i.e. basically identical except verdaccio url encodes the scope/package as @babel%2fcode-frame instead of @babel/code-frame

But yarn's regex for finding packages in its offline cache don't work with this: https://github.com/yarnpkg/yarn/blob/master/src/fetchers/tarball-fetcher.js#L21

To Reproduce
Steps to reproduce the behavior:

  1. yarn add a scoped package
  2. follow steps here to populate offline mirror

Expected behavior
Offline mirror should have all the scoped packages.

Is there a reason why verdaccio url encodes the / there?

@moon-stripe
Copy link
Author

I found https://github.com/verdaccio/verdaccio/blob/master/src/api/middleware.js#L73

I assume this is to avoid writing extra routes for scoped packages, but if that's the only concern, would you accept a PR to remove that middleware and add the extra routes necessary to preserve the /?

@juanpicado
Copy link
Member

juanpicado commented May 17, 2019

@moon-stripe, to be honest, this was done even before me as a contributor, so I'd need to find out the reason of the encoded URL first. Other question might be how pnpm or npm are handling this?

@moon-stripe
Copy link
Author

@juanpicado afaict, npm doesn't have an equivalent offline mode. I'm not sure how pnpm's offline mode interacts. But in general, it seems best to mimc upstream npm behavior as closely as possible so as to not behaviors such as yarn's offline caching.

@juanpicado
Copy link
Member

Is there a reason why verdaccio url encodes the / there?

Perhaps, that's legacy code. So, first, we need to know why is encoded and whether either yarn or use owns the bug.

@juanpicado
Copy link
Member

@moon-stripe
Copy link
Author

@juanpicado Interesting, thanks for those links! Reading through the thread and looking at the commits, it looks like it was simply the most expedient way to support scoped packages? And it seems to have caused issues with using an nginx proxy, which required some special-casing.

@juanpicado
Copy link
Member

Exactly, so, what I'm not sure is whether that still required or not, we have some examples with NGINX https://github.com/verdaccio/docker-examples/tree/master/reverse_proxy/nginx and should be easy to try.

@DanielRuf
Copy link
Contributor

cc @zkochan what would you recommend?

@zkochan
Copy link
Collaborator

zkochan commented May 27, 2019

I think it would be better if Verdaccio would use the same URL format as the npmjs.org. But pnpm works fine with the encoded URL format. pnpm normalizes the URL before saving it to the cache.

EDIT:

pnpm uses this package to parse a npm tarball URL. And here's the test for this specific case.

@DanielRuf
Copy link
Contributor

Only yarn seems to have an issue with this.
So should we better open an issue for yarn?

@juanpicado
Copy link
Member

@DanielRuf @zkochan I think he is aware https://twitter.com/arcanis/status/1131121028573388801

@juanpicado
Copy link
Member

I think this can be closed yarnpkg/yarn@b8af7e0
yarnpkg/yarn#7499
@moon-stripe please try again or :-) just tell me so I can close this ticket.

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

No branches or pull requests

4 participants