volta-cli / volta

Volta: JS Toolchains as Code. ⚡
https://volta.sh
Other
10.92k stars 227 forks source link

`volta list` output is wrong for locally-installed tools #711

Open chriskrycho opened 4 years ago

chriskrycho commented 4 years ago

In a project with a given tool installed locally (in this case, ember-cli)—

Additionally, we currently report the wrong platform values for these. In the extended form (volta list all):

This correctly reports the Node version (though without the current @ ... from the design), but not the correct package manager version.

This affects both the --format=plain and the --format=human variants, which is expected: the underlying logic for both is identical.

charlespierce commented 4 years ago

The 2nd part of this is potentially tricky for a couple of reasons: First, we don't have an obvious way to determine the version of a local binary: It would be equally wrong to show ember-cli@3.17.0 (current @ ...) when the local project is actually using ember-cli@3.12.0. The only way I can think of to determine that would be to dig into node_modules/ember-cli/package.json and read the version from there (we can't use the project's package.json because it could be a semver range).

Additionally, the logic for determining the platform that a binary will use is currently bespoke logic inside of volta_core::run::binary, so in order to duplicate it we would probably want to refactor it out into somewhere reusable. That would actually simplify a few other things (like the volta run implementation), so it's worth doing, but it's not a super simple change.

chriskrycho commented 4 years ago

I was thinking about that first part, too, but hadn't worked out the details on the second part yet. That's tricky indeed. 🤔

Right now, we're printing something that's actually out-and-out wrong, so I'm wondering if we shouldn't tackle this in two phases:

  1. Don't print the platform or version info for it at all for local binaries; just print ember-cli (current @ ...) or something. That gives the user enough info to introspect the version manually, while not printing erroneous information.

  2. Design a solution which correctly handles the bits to go ahead and supply that data correctly after landing volta run.

Thoughts? cc. @rwjblue @dherman

charlespierce commented 4 years ago

I missed this comment when the issue first came up, but I agree. Since we don't have a way forward yet on showing the correct version, we should at the very least prevent showing incorrect information for now.

charlespierce commented 4 years ago

I had a thought on how to determine the version: We could display the value from package.json directly, including any semver ranges. So if they have "typescript": "^3.0.0", we would show typescript@^3.0.0, which is valid npm-style syntax and is accurate (though not specific). For determining the platform we can definitely do that, it'll just take some refactoring on the run::binary module to abstract out that logic.

TheBrenny commented 1 month ago

Out of curiosity, why isn't it okay to simply rummage through to node_modules/ember-cli/package.json to grab the version? Afai can see, that's likely the only method that's feasible...