feat: reads versions from lock files

[jmcdo29]

Reading from the appropriate lock files instead of the
package.json means having a better idea of what packages
are installed. With the package.json, there's a chance to
miss the actual installed version due to using a range
like ^7.0.0 whereas with the lock file, the version is
printed to what the package manager resolved.

To make this more easily achievable, I refactored the
info.action.ts file to be more class based so I could
reuse the package manager instead of having to re-find it
each time.

re: #896

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
  Tests didn't already exist, though I'm happy to try and write some up
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Currently we are reading the nest package versions from the package.json.
Issue Number: #896

What is the new behavior?

After this PR we will read them from the appropriate lock file.

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Kind of depends on your definition of breaking. Though it should be noted that this PR doesn't support PNPM as we need to merge #788 for that. Because of this, PNPM users will lose access to nest info until that can be taken up, or we have the old functionality still in place (for pnpm and for not finding a lock file).

Other information

[jmcdo29]

I just realized as well the error message still references package.json so I'll need to update that before the PR is merged.

[dkantereivin]

Kind of depends on your definition of breaking. Though it should be noted that this PR doesn't support PNPM as we need to merge #788 for that. Because of this, PNPM users will lose access to nest info until that can be taken up, or we have the old functionality still in place (for pnpm and for not finding a lock file).

Regardless of #788, keeping the legacy functionality is a fallback for unrecognized lockfiles seems advantageous. As Kamil mentions in #788, nest-cli is theoretically package-manager-agnostic in that any package manager can be used (by manually deleting node_modules and lockfile and installing packages).

This also means that if nobody were to add pnpm lockfiles, then pnpm users would at least get some nest info functionality. This reduces the maintenance cost of pnpm. That being said, I get the feeling that @jmcdo29 would be willing to add this functionality (and if not, I would).

[dkantereivin]

Insignificant, but I think this would be more clear as readProjectLockfileDependencies.

[jmcdo29]

Probably, but I was also trying to keep the name from getting too long.

[dkantereivin]

I agree, perhaps it's worth including this as a comment? Just because, a "project lock" doesn't exactly refer to anything at all.

[jmcdo29]

I think the fact that 3 lines later you see package-lock.json or yarn.lock makes the intention clear. Though I guess in a stack trace it could be somewhat problematic, though a comment wouldn't help that either

[dkantereivin]

What if you removed the word project? It's probably the first assumption once you see "lockfile," although I suppose it doesn't have to be a project.

[kamilmysliwiec]

Thanks @jmcdo29! Can we fallback to package.json (and information about the dependencies from this file) if any error is thrown? In this case, we shouldn't introduce any breaking change for anyone who is using a different package manager than npm or yarn. Also, how complicated would it be to integrate pnpm as well?

[andreialecu]

It would probably be much simpler and less problematic to use require("@nestjs/cli/package.json").version to get the right version instead of trying to read lockfiles. And it will work even without having a lockfile, or with any other package manager.

Maybe require.resolve with the paths option if it needs to be resolved from a different folder, then either read package.json with require or with readFileSync and JSON.parse (safer)

[jmcdo29]

Thanks @jmcdo29! Can we fallback to package.json (and information about the dependencies from this file) if any error is thrown? In this case, we shouldn't introduce any breaking change for anyone who is using a different package manager than npm or yarn.

That's the current functionality. If there's an error that happens while reading the lockfile, the package.json is used instead

Also, how complicated would it be to integrate pnpm as well?

I'd have to look into the pnpm-lock.yaml file and see how to parse that. I can say at the moment that I've tested the current code against a pnpm nest project and it does currently read the package.json

[jmcdo29]

It would probably be much simpler and less problematic to use require("@nestjs/cli/package.json").version to get the right version instead of trying to read lockfiles. And it will work even without having a lockfile, or with any other package manager.

Maybe require.resolve with the paths option if it needs to be resolved from a different folder, then either read package.json with require or with readFileSync and JSON.parse (safer)

Hmm, not a bad idea actually. We'd need to keep a running list of all Nest package in the CLI repo. Or we could read the dependencies and devDependencies of the package.json and then find all @nestjs/ dependencies, iterate over them and use the require(package).version as you mentioned.

[andreialecu]

Just a note, it is theoretically better to use fs.readFile and JSON.parse than to require package.json, since in theory package.json.js might be executable, and we wouldn't want to execute that. It shouldn't happen unless some sort of malware does it, but still, better to read and parse.

Ugly one liner: JSON.parse(fs.readFileSync(require.resolve("@nestjs/cli/package.json", { paths: [process.cwd()] }))).version

[jmcdo29]

Unfortunately had to force push due to some rebasing problems, but got the update done which uses fs.readFileSync instead of require. Also, there's no more lock file dependency so this works well with any package manager (at least the big three had no problem). Let me know if you have any other concerns

[jmcdo29]

@kamilmysliwiec Any way we can make this a part of the v8 push? Just to have better package version reporting by the nest info command?

[kamilmysliwiec]

LGTM 🙌 Apologies for the delay