fix: Adds support for Yarn 2 #757
Conversation
|
I think this should probably have tests but I couldn't see anywhere to add them easily. Any thoughts? It also could just... not be tested... but it probably should be. :) I'll try to build on replit at a minimum... will do sometime this week probably. @joaomoreno pinging you since you own #517... do you have test opinions? |
Not specific ones... just that it should probably be tested. ;) |
| (err, stdout) => (err ? e(err) : c(stdout)) | ||
| ) | ||
| ); | ||
| const yarnCommand = versionString[0] !== 1 ? "yarn info --recursive --dependents --json" : 'yarn list --prod --json' |
There was a problem hiding this comment.
| const yarnCommand = versionString[0] !== 1 ? "yarn info --recursive --dependents --json" : 'yarn list --prod --json' | |
| const yarnCommand = versionString[0] !== "1" ? "yarn info --recursive --dependents --json" : 'yarn list --prod --json' |
The versionString[0] !== 1 part causes a compilation error:
src/npm.ts:165:22 - error TS2367: This condition will always return 'true' since the types 'string' and 'number' have no overlap.
| const raw = await new Promise<string>((c, e) => | ||
| cp.exec( | ||
| 'yarn list --prod --json', | ||
| yarnCommand, |
There was a problem hiding this comment.
Line 176 (where it says Could not parse result of […]) should be updated with yarnCommand as well; otherwise the error message is going to be misleading.
| const raw = await new Promise<string>((c, e) => | ||
| cp.exec( | ||
| 'yarn list --prod --json', | ||
| yarnCommand, |
There was a problem hiding this comment.
Are you sure that yarn info --recursive --dependents --json is a compatible drop-in replacement for yarn list --prod --json, anyway?
On Yarn v3.3.1, the former prints a list of JSON objects, for example:
{"value":"yallist@npm:4.0.0","children":{"Version":"4.0.0","Dependents":["lru-cache@npm:6.0.0","minipass@npm:3.3.6","minipass@npm:4.0.0","minizlib@npm:2.1.2","tar@npm:6.1.13"]}}At least on Yarn 3.3.1, none of those lines seem to contain anything like "type":"tree". However, the code that follows seems to rely on just that. Consequently, vsce package now errors out for me in line 176, saying:
Could not parse result of `yarn list --json`
Closes #517