Regression in v1.11.0: detected package manager defaults to yarn

[waldyrious]

The changes introduced in #744 and #765 result in the use of the incorrect package manager in some cases, where yarn is used where npm should be used instead.

For context, I am working in a repository that has a package-lock.json rather than a yarn.lock file; and indeed, my team uses npm instead of yarn. However, when we added postman-code-generators as a dependency, we started getting errors in CI, because we use an internal npm registry, and yarn is not able to access it using the same configurations and authentication data as npm.

For some reason, neither our top-level package-lock.json, nor this package's own package-lock.json are triggering the first condition that detect-package-manager was supposed to check (the presence of a given package manager's lockfile), so it goes to step 2, which is to check what package managers are available in the machine. And indeed the CI runner we use has both npm and yarn available.

Unfortunately, in the presence of both npm and yarn, detect-package-manager gives precedence to yarn, for some reason. (This has been reported in egoist/detect-package-manager#11, but after a year, there's no resolution yet — or even acknowledgement of the issue by the developer, for that matter).

@aman-v-singh, do you think it would be possible to reorder the array returned by detect-package-manager's detect(), before it is used in npm/deepinstall.js, so that npm comes first in the list?

[aman-v-singh]

Hey,
Thank you for reporting this.
@waldyrious Can you please share the error log along with the version of the package managers so I can understand why the first condition might not be satisfied?

This issue might occur because the installation command is not run from the same directory where the package-lock.json file exists.

[waldyrious]

Thanks for responding!

The project in question relies on Java dependencies, and therefore it uses Maven to orchestrate the build, including the step of running the npm install command (which, when it comes to the postman-code-generators dependency, runs the npm/deepinstall.js script which is what's swapping to yarn instead).

I'm not sure if this is relevant to why detect-package-manager isn't identifying the package-lock.json file, but I thought I'd mention it just in case, because (1) I do know that Maven spawns commands in isolated shell environments (but not isolated working directories AFAIK), and (2) it explains why the log entries below are prefixed with XX:XX:XX [INFO] [npm|docusaurus]

Here's the relevant section of the CI job log:

11:41:03 [INFO] --- docusaurus-maven-plugin:1.3.0:clean (build-docusaurus) @ product-documentation ---
11:41:33 [INFO] [npm|docusaurus] Executing command npm install
(some unrelated npm package deprecation warnings omitted)
11:42:20 [INFO] [npm|docusaurus] npm error code 1
11:42:20 [INFO] [npm|docusaurus] npm error path /builds/product-docs/library/node_modules/postman-code-generators
11:42:20 [INFO] [npm|docusaurus] npm error command failed
11:42:20 [INFO] [npm|docusaurus] npm error command sh -c node npm/deepinstall.js
11:42:20 [INFO] [npm|docusaurus] npm error Detected package manager: yarn
11:42:20 [INFO] [npm|docusaurus] npm error Detected yarn version: 1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error Running pre-package script
11:42:20 [INFO] [npm|docusaurus] npm error Run successful languages.js saved in lib/assets
11:42:20 [INFO] [npm|docusaurus] npm error csharp-httpclient: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error yarn install v1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error info No lockfile found.
11:42:20 [INFO] [npm|docusaurus] npm error [1/5] Validating package.json...
11:42:20 [INFO] [npm|docusaurus] npm error [2/5] Resolving packages...
11:42:20 [INFO] [npm|docusaurus] npm error [3/5] Fetching packages...
11:42:20 [INFO] [npm|docusaurus] npm error [4/5] Linking dependencies...
11:42:20 [INFO] [npm|docusaurus] npm error [5/5] Building fresh packages...
11:42:20 [INFO] [npm|docusaurus] npm error Done in 0.14s.
11:42:20 [INFO] [npm|docusaurus] npm error
11:42:20 [INFO] [npm|docusaurus] npm error csharp-restsharp: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error yarn install v1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error info No lockfile found.
11:42:20 [INFO] [npm|docusaurus] npm error [1/5] Validating package.json...
11:42:20 [INFO] [npm|docusaurus] npm error [2/5] Resolving packages...
11:42:20 [INFO] [npm|docusaurus] npm error [3/5] Fetching packages...
11:42:20 [INFO] [npm|docusaurus] npm error [4/5] Linking dependencies...
11:42:20 [INFO] [npm|docusaurus] npm error [5/5] Building fresh packages...
11:42:20 [INFO] [npm|docusaurus] npm error Done in 0.15s.
11:42:20 [INFO] [npm|docusaurus] npm error
11:42:20 [INFO] [npm|docusaurus] npm error curl: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error yarn install v1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error info No lockfile found.
11:42:20 [INFO] [npm|docusaurus] npm error [1/5] Validating package.json...
11:42:20 [INFO] [npm|docusaurus] npm error [2/5] Resolving packages...
11:42:20 [INFO] [npm|docusaurus] npm error [3/5] Fetching packages...
11:42:20 [INFO] [npm|docusaurus] npm error [4/5] Linking dependencies...
11:42:20 [INFO] [npm|docusaurus] npm error [5/5] Building fresh packages...
11:42:20 [INFO] [npm|docusaurus] npm error Done in 0.14s.
11:42:20 [INFO] [npm|docusaurus] npm error
11:42:20 [INFO] [npm|docusaurus] npm error dart-dio: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error dart-http: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error yarn install v1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error info No lockfile found.
11:42:20 [INFO] [npm|docusaurus] npm error [1/5] Validating package.json...
11:42:20 [INFO] [npm|docusaurus] npm error [2/5] Resolving packages...
11:42:20 [INFO] [npm|docusaurus] npm error [3/5] Fetching packages...
11:42:20 [INFO] [npm|docusaurus] npm error [4/5] Linking dependencies...
11:42:20 [INFO] [npm|docusaurus] npm error [5/5] Building fresh packages...
11:42:20 [INFO] [npm|docusaurus] npm error Done in 0.14s.
11:42:20 [INFO] [npm|docusaurus] npm error
11:42:20 [INFO] [npm|docusaurus] npm error golang: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error yarn install v1.22.22
11:42:20 [INFO] [npm|docusaurus] npm error info No lockfile found.
11:42:20 [INFO] [npm|docusaurus] npm error [1/5] Validating package.json...
11:42:20 [INFO] [npm|docusaurus] npm error [2/5] Resolving packages...
11:42:20 [INFO] [npm|docusaurus] npm error [3/5] Fetching packages...
11:42:20 [INFO] [npm|docusaurus] npm error [4/5] Linking dependencies...
11:42:20 [INFO] [npm|docusaurus] npm error [5/5] Building fresh packages...
11:42:20 [INFO] [npm|docusaurus] npm error Done in 0.14s.
11:42:20 [INFO] [npm|docusaurus] npm error
11:42:20 [INFO] [npm|docusaurus] npm error http: yarn install --production --frozen-lockfile
11:42:20 [INFO] [npm|docusaurus] npm error Failed to run yarn install on codegen dart-dio, here is the error:
11:42:20 [INFO] [npm|docusaurus] npm error warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
11:42:20 [INFO] [npm|docusaurus] npm error error Error: https://maven.example.com/repository/npm-all/lodash/-/lodash-4.17.21.tgz: Request failed "401 Unauthorized"

Does this help in any way?

[aman-v-singh]

@waldyrious I can see here that some of the codegens are getting installed successfully, the error is happening in codegens that contain a package-lock.json of their own.

This issue can be fixed by removing the package-lock.json files, for which I have made a PR.
This change will possibly fix the problem, although the installation will still be happening with yarn because of detect-package-manager.

To get detect-package-manager to use npm either remove yarn from the global scope or add a package-lock.json file at the location from where the node run deepinstall command is being run.

[waldyrious]

Thanks, @aman-v-singh!

To get detect-package-manager to use npm either remove yarn from the global scope or add a package-lock.json file at the location from where the node run deepinstall command is being run.

To be honest, I think that even though this is an issue with detect-package-manager, postman-code-generators should not blindly follow its lead and use yarn when npm is available, requiring workarounds by its users to bypass this behavior — not just as a matter of principle, but especially considering that it results in issues like this.

Until the ordering of the results of detect() is fixed in detect-package-manager (egoist/detect-package-manager#11), IMHO postman-code-generators should place npm at the start of the list of available package managers, and therefore prefer its usage over yarn. After all, it is a deliberate decision for postman-code-generators to use detect-package-manager and propagate its behavior downstream, so the responsibility should lie in postman-code-generators to uphold the principle of least surprise and ensure the behavior is appropriate given its own code and that of its dependencies. Don't you agree?

[aman-v-singh]

@waldyrious As my priority I have released a new patch version v1.14.1 which I think would fix your issue.
I feel for the sake of the installation of codegens, the package manager would not matter, and your project should work fine even with yarn installation. Can you please check, if the latest version fixes your issue?

I think that even though this is an issue with detect-package-manager, postman-code-generators should not blindly follow its lead and use yarn when npm is available

detect-package-manager checks for the presence of a lock file for corresponding package managers (question of priority does not come here) and when it is not present it checks for globally available package managers. Now, the reason why I think it prioritizes other package managers over npm here is that it was created for users who would like to have their projects supported by other package managers. It was for the same reason we used this in our project. As npm is already the most popular package using something like detect-package-manager to prioritise npm does not make sense.

So we can also take up writing our utilities to detect package managers where we can change the priority. It can be a task for the next minor release.

I can see the upside of prioritizing yarn or npm, but to completely avoid this, having a lock file of the corresponding package manager in the installation directory can be the best solution, as it is the surest way to detect the correct package manager. The lock file can even be empty with a comment stating its use.

Thoughts? @VShingala

[waldyrious]

@aman-v-singh interestingly, the changes from 1.14.1 do fix the issue! Thanks a lot for the quick reaction! 😀

So just to be sure I understand correctly, what is happening now is that npm/deepinstall.js is still using yarn to install those codegen packages, but it's no longer erroring out because there are no npm lockfiles there, right?

I still think it might be worth investigating why the presence of the package-lock.json files wasn't leading detect-package-manager to opt for npm. After all, package-lock.json files are npm-specific, so if anything, they should have been leading detect-package-manager to pick npm over yarn, not the other way around.

While usage of yarn does not cause an error anymore, it is still not what one would expect given the code, and may cause hard-to-debug issues down the road, so you may want to keep this issue open until that conundrum is figured out. That said, if you'd prefer closing the issue and letting it be, that's fine by me.

[aman-v-singh]

So just to be sure I understand correctly, what is happening now is that npm/deepinstall.js is still using yarn to install those codegen packages, but it's no longer erroring out because there are no npm lockfiles there, right?

Correct!

I still think it might be worth investigating why the presence of the package-lock.json files wasn't leading detect-package-manager to opt for npm. After all, package-lock.json files are npm-specific, so if anything, they should have been leading detect-package-manager to pick npm over yarn, not the other way around.

@waldyrious The package-lock.json file must be at the root of your project (or from wherever the installation of dependencies gets triggered in your project). Your project directory might look something like this:-

project-root/
├── src/
│   └── main/
│       ├── java/
│       └── resources/
├── frontend/
│   ├── package.json
│   └── node_modules/
├── package-lock.json -> Here
└── pom.xml

The package-lock.json files that were removed were from individual codegens, ideally, these should have never been committed but these didn't cause an issue as long as only npm was being used.

While usage of yarn does not cause an error anymore, it is still not what one would expect given the code, and may cause hard-to-debug issues down the road, so you may want to keep this issue open until that conundrum is figured out. That said, if you'd prefer closing the issue and letting it be, that's fine by me.

I will close this issue for now but will discuss it with my team and look for a more reliable solution for this problem.