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

Yarn support #30

Closed
hassankhan opened this issue Feb 8, 2017 · 14 comments
Closed

Yarn support #30

hassankhan opened this issue Feb 8, 2017 · 14 comments
Labels

Comments

@hassankhan
Copy link
Contributor

hassankhan commented Feb 8, 2017

This is a Feature Proposal

Description

I created a .npmrc in my home folder, then tried to use yarn publish in a project.

  • What went wrong?

I got the following in my terminal:

yarn publish v0.19.1
[1/4] Bumping version...
info Current version: 1.0.3
question New version: 1.0.4
info New version: 1.0.4
[2/4] Logging in...
question npm username: hassankhan.otpotp
question npm email: [email protected]
question npm password:
success Logged in.
[3/4] Publishing...
success Published.
[4/4] Revoking token...
success Revoked login token.
error An unexpected error occurred: "https://l90d4v81rh.execute-api.eu-west-1.amazonaws.com/prod/registry/-/user/token/<myToken>: '<myToken>' not a valid key=value pair (missing equal-sign) in Authorization header: 'Bearer <myToken>'.".
info If you think this is a bug, please open a bug report with the information provided in "/Users/hassankhan/Projects/yith-test/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/publish for documentation about this command.
  • What did you expect should have happened?

It should have published like npm publish would have.

Additional Data

  • NPM CLI version you are using: 3.10.3
  • Yarn CLI version you are using: 0.19.1
  • Serverless version you're using: 1.6.1
  • Node version you're using: 6.7.0
@jonsharratt
Copy link
Member

This is something I am actively trying to figure out. Seems yarn doesn't pick up the .npmrc file correctly with the authToken or at least so far with my investigations it doesn't appear to.

The plan is to create an integration test to fill in these tests so I can start supporting current packager manager CLIs as well as future ones.

If you do try out the commands and get the results as per above please do post them as individual issues and we can try and crack on fixing them or finding a way to get it to be compatible.

@hassankhan
Copy link
Contributor Author

hassankhan commented Feb 8, 2017

You're completely right, this is actually Yarn's fault for not reading from my .npmrc. Might be better to just keep an eye on Yarn's changelogs 😄

EDIT:

Found some relevant issues:
yarnpkg/yarn#2193
yarnpkg/yarn#2508
yarnpkg/yarn#2541

@panva
Copy link

panva commented Feb 9, 2017

yarn is ignoring authToken for unscoped packages. I have it working okay with other private registry software (sinopia) where we publish all our internal (private) projects with @xxx/name scheme and have all unscoped packages our projects depend on pulled/proxied by this registry as public.

There is little you can do on your end to fix this.

@brimworks
Copy link

Yarn works fine if you manually modify index.json stored in S3 so that the dist.tarball begins with "https://" instead of "http://". Seems like this is a bug with npm publish not setting the tarball url properly (dropping the "s"), or you can replace "http://" simply with "//".

Perhaps a work-around to this problem is to modify put.js so it removes the http: prefix?

@brimworks
Copy link

...actually it appears that yarn will publish with this field set to registry.yarnpkg.com/... so we might want to blindly overwrite this field (maybe even in the get.js) so that yarn and npm always fetch the tarball from yith.

@brimworks
Copy link

I was wrong about "//" (no protocol) URLs working in the tarball field. Seems that we always need it to be https://.

@jonsharratt
Copy link
Member

Awesome, thanks for taking a look into yarn @brimworks. Spooky, we ran across an unrelated issue today in which we need to ensure the urls https no matter what (should be using https regardless 🙈).

Will do this as the first fix, great spot!

Going to add as a separate feature, the creation of a migration tool for domains so that users can update domains / change them easily. I feel this will become a common use case when you want to add a custom domain and have already published packages on the default API gateway URL.

Let me know if you find any other odd behaviour with yarn as would be good to stick a stamp of approval that this project can support it fully.

👍

@brimworks
Copy link

This is the change I ended up making to get it to play nice with yarn:

diff -r ba95285a8819 yith/src/get.js
--- a/yith/src/get.js	Tue Feb 28 14:11:12 2017 -0800
+++ b/yith/src/get.js	Wed Mar 01 10:51:21 2017 -0800
@@ -13,11 +13,28 @@

   const name = `${decodeURIComponent(event.pathParameters.name)}`;

+  const registryBase =
+    event.headers["X-Forwarded-Proto"] + "://" +
+    event.headers["Host"] + "/" +
+    event.requestContext["stage"] + "/registry/";
   try {
     const pkgBuffer = await storage.get(`${name}/index.json`);
     const json = JSON.parse(pkgBuffer.toString());
     json._attachments = {}; // eslint-disable-line no-underscore-dangle

+    if ( json.versions ) {
+      Object.keys(json.versions).forEach((versionName) => {
+        let version = json.versions[versionName];
+        if ( version.dist && version.dist.tarball ) {
+            // Fix tarball to always point back to this install:
+            let components = version.dist.tarball.split('/registry/');
+            if ( components.length > 1 ) {
+                version.dist.tarball = registryBase + components[components.length-1];
+            }
+        }
+      });
+    }
+
     return callback(null, {
       statusCode: 200,
       body: JSON.stringify(json),
diff -r ba95285a8819 yith/src/put.js
--- a/yith/src/put.js	Tue Feb 28 14:11:12 2017 -0800
+++ b/yith/src/put.js	Wed Mar 01 10:51:21 2017 -0800
@@ -67,8 +67,7 @@
   const versionData = pkg.versions[version];

   const tarballFilename = encodeURIComponent(versionData.dist.tarball.split('/-/')[1]);
-  const tarballBaseUrl = versionData.dist.tarball.split('/registry/')[0];
-  versionData.dist.tarball = `${tarballBaseUrl}/registry/${pathParameters.name}/-/${tarballFilename}`;
+  versionData.dist.tarball = `/registry/${pathParameters.name}/-/${tarballFilename}`;

   let json = {};

The advantage of the above implementation is there is no need to "migrate" existing index.json files, since we process the json in the GET.

@jonsharratt
Copy link
Member

Not 100% sure how you resolved the auth issue, I presume you are using scoped packages as @panva mentioned to ensure the auth token gets sent.

Having a quick play with yarn has also uncovered a change in regards to the way yarn sets up login, will need to shift OTP for 2FA of GitHub to the password field, it should really be there anyway.

Currently I can't quite get it to play nice at all with a scoped registry, GitHub and 2FA.

Thanks for the code snippet but would much rather get the root cause resolved. The migration script will have to be done regardless as we already would have a use for it currently.

I will also schedule some time to do some more thorough testing with yarn to see what other amends need to be done other than the OTP.

@brimworks
Copy link

@jonsharratt no need to use scoped packages. The latest yarn version 0.21.3 worked out-of-the box for me. I did not use 2FA, but here is how I set-up things:

[a] I deployed yith 0.9.0 with the above patch.

[b] In my local dev box I updated to yarn version 0.21.3.

[c] I ran (npm version 4.0.5):

npm set registry https://XXX.execute-api.us-west-2.amazonaws.com/STAGE/registry/
npm login

Note that ~/.yarnrc is pretty much empty, and ~/.npmrc just has the registry and auth token.

Not sure about scoped registries, as I simply ran yarn publish on a non-scoped registry and it worked flawlessly (after my above change). I later consumed it in another package and was successfully able to pull the package with yarn install.

@jonsharratt
Copy link
Member

So to further to this, having done a bit of testing with #48 as @panva mentioned it will work with scoped packages as yarn will send the auth token. Without scopes it does not appear to, I am on the same version of yarn as yourself.

Current investigations mean that:

  • Installing a scoped package works but not with it's dependencies that that are not scoped (no auth header token sent).
  • Publishing works fine as long as they are scoped package names.

Need to look into the related issues and look at yarn to understand whether it will ever be possible to support it.

@brimworks
Copy link

Yikes, I omitted one critical piece of information. You need to add this to the end of your .npmrc for it to work with non-scoped packages:

always-auth=true

@jonsharratt
Copy link
Member

jonsharratt commented Mar 2, 2017

Amazing @brimworks! yarn add now works 👍 one to note @panva, might be the same deal for Sinopia.

Will continue testing tomorrow and then it looks like yarn is supported.

Thanks for all your assistance on this, so cool to get this on confirmed and working.

@jonsharratt jonsharratt mentioned this issue Mar 3, 2017
4 tasks
@panva
Copy link

panva commented Mar 3, 2017

@brimworks #30 (comment) great!

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

No branches or pull requests

4 participants