Inconsistent quoting of github dependency names leads to unnecessary lockfile changes

[Gekkio]

Do you want to request a feature or report a bug?

This seems like a yarn bug.

What is the current behavior?

We've got a certain transitive dependency that is always resolved correctly, but the name of this dependency is sometimes quoted depending on which yarn command was executed last.

If I run yarn install in our project, yarn.lock always ends up containing this (dependency name not quoted):

axios@contentful/axios#fix/https-via-http-proxy:
  version "0.17.1"
  resolved "https://codeload.github.com/contentful/axios/tar.gz/4b06f4a63db3ac16c99f7c61b584ef0e6d11f1af"
  dependencies:
    follow-redirects "^1.2.5"
    is-buffer "^1.1.5"

If I run yarn upgrade in our project, yarn.lock always ends up containing this (dependency name quoted):

"axios@github:contentful/axios#fix/https-via-http-proxy":
  version "0.17.1"
  resolved "https://codeload.github.com/contentful/axios/tar.gz/4b06f4a63db3ac16c99f7c61b584ef0e6d11f1af"
  dependencies:
    follow-redirects "^1.2.5"
    is-buffer "^1.1.5"

So, even if there are no actual version changes, install and upgrade can cause unnecessary lockfile changes that the other command reverts later. Removing node_modules doesn't help.

If the current behavior is a bug, please provide the steps to reproduce.

This is reproducible with a barebones package.json with these two dependencies:

"dependencies": {
  "contentful": "~4.6.2",
  "left-pad": "stevemao/left-pad"
}

Now, running yarn upgrade quotes the axios transitive dependency, and yarn install reverts the quoting. This seems to have something to do with multiple github dependencies, because if left-pad is removed, both yarn commands use quotes in the axios dependency name. Note that contentful doesn't use left-pad even transitively, so just the existence of another github dependency is enough to trigger this behaviour.

What is the expected behavior?

yarn.lock uses consistently either the quoted or non-quoted naming for axios, so install/upgrade doesn't do unnecessary quoting modifications to the lock file.

Please mention your node.js, yarn and operating system version.

$ node --version
v8.9.1
$ yarn --version
1.3.2
$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=17.04
DISTRIB_CODENAME=zesty
DISTRIB_DESCRIPTION="Ubuntu 17.04"

[rally25rs]

They are actually different:

axios@contentful/axios#fix/https-via-http-proxy
axios@github:contentful/axios#fix/https-via-http-proxy

The : causes it to need to be wrapped in double-quotes.

---

Still stepping through this, but it seems like:

• on initial install, it resolves the pattern axios@contentful/axios#fix/https-via-http-proxy
• on upgrade, no packages are out of date, so add is called with an empty array of packages to upgrade. This time around it resolves the pattern axios@github:contentful/axios#fix/https-via-http-proxy

Both code paths pass through src/cli/commands/install.js to do their work, but the this.resolver.patterns has the above difference between the two runs.

[rally25rs]

Digging further... I think this might have something to do with the dependencies being different between the contentful packages registry entry and it's package.json...

If you look at what the NPM registry returns for http://registry.npmjs.org/contentful under versions['4.6.4].dependencies.axios you get github:contentful/axios#fix/https-via-http-proxy

However in the downloaded .tgz file's package.json dependencies.axios you get: contentful/axios#fix/https-via-http-proxy

---

I'm not entirely sure, but I think when there is a cache hit on install it uses the package.json entry, and during upgrade cache is ignored (because it doesn't really make sense to upgrade while offline) and it uses the NPM registry entry.

---

I'm not really sure where else to go with this one... any help is appreciated. @BYK @kaylieEB do either of you know if this difference in registry vs package.json dependencies is a knows NPM-oddity?

[BYK]

@BYK @kaylieEB do either of you know if this difference in registry vs package.json dependencies is a knows NPM-oddity?

I think this can be the case all the time. I feel like package.json should take precedence (which I think is what's happening here). That said in that case what we resolve would be different from what we got. Maybe we can simply fail and throw early if we detect this when we download the package and also remove it from the cache?

[ooooseaoooo]

In my Expo project, Whenever I ran npm install I got tons of following changes due to inconsistent quotes.

-  version "7.13.8"
-  resolved "https://registry.npmjs.org/@babel/plugin-proposal-optional-chaining/-/plugin-proposal-optional-chaining-7.13.8.tgz"
-  integrity sha512-hpbBwbTgd7Cz1QryvwJZRo1U0k1q8uyBmeXOSQUjdg/A2TASkhR/rz7AyqZ/kS8kbpsNA80rOYbxySBJAqmhhQ==
+  "integrity" "sha512-hpbBwbTgd7Cz1QryvwJZRo1U0k1q8uyBmeXOSQUjdg/A2TASkhR/rz7AyqZ/kS8kbpsNA80rOYbxySBJAqmhhQ=="
+  "resolved" "https://registry.npmjs.org/@babel/plugin-proposal-optional-chaining/-/plugin-proposal-optional-chaining-7.13.8.tgz"
+  "version" "7.13.8"

[BreakUps]

@ooooseaoooo Maybe use yarn install avoids the changes. yarn.lock file generated by npm install differs.