-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add yarn v2+ support #39
Conversation
resolves #32
package.json
Outdated
@@ -47,6 +47,7 @@ | |||
}, | |||
"dependencies": { | |||
"kolorist": "^1.8.0", | |||
"latest-version": "^5.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pulls in a total of 19 dependencies to do a simple fetch call, see https://npm.anvaka.com/#/view/2d/latest-version . That's a bit much to fetch some json from the npm api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep i agree, added this just to get it to work but will look into replacing this with something better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've changed it now so it pulls the latest version from the meta.json
given by the jsr using fetch. let me know if you think it's a good idea or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start, thanks for tackling this 👍 Left some minor comments. Let me know what you think.
src/pkg_manager.ts
Outdated
function modeToFlagYarn(mode: InstallOptions["mode"]): string { | ||
return mode === "dev" | ||
? "--dev" | ||
: mode === "optional" | ||
? "--optional" | ||
: ""; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yarn berry does not respect the --save-dev
and --save-optional
flags unlike npm + pnpm. bun also has the same problem, it only understands the yarn style --dev
and --optional
flags but it's arguably worse than yarn as it doesn't throw errors when an unrecognized flag is passed in. additionally, if bun supported the shorthand for --optional
(-O
) then we wouldn't need this at all as all the package managers currently understand the shorthand form for installing a dev dependency (-D
).
CI failure is unrelated to the changes in this PR. Not sure why it's different on windows yet. |
maybe not actually, it's resolving |
i added a potential fix to wrap the command arguments in double quotes on windows (works for both powershell and cmd) and single quotes elsewhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, thank you so much for adding this! 🙌
resolves #32