-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix: Run dedupe during upgrade on yarn 3 #5458
fix: Run dedupe during upgrade on yarn 3 #5458
Conversation
✅ Deploy Preview for redwoodjs-docs canceled.
|
I think yarn 3 automatically does deduping though, doesn't it? |
[...npxArgs, 'yarn-deduplicate'], | ||
baseExecaArgsForDedupe | ||
) | ||
} | ||
} catch (e) { | ||
console.log(c.error(e.message)) | ||
throw new Error( |
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.
maybe we can print the correct command based on the yarn version?
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.
We can but if for some reason regex fails for semver, that's why I kept it one simple catch.
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.
The one catch is fine, but just suggesting printing different messages
if (yarnVersion > 1) {
console.log('Failed dedupe step. Run yarn dedupe')
} else {
console.log('Failed dedupe. run npx yarn-deduplcate')
}
if (yarnVersion > 1) { | ||
await execa('yarn', ['dedupe'], baseExecaArgsForDedupe) | ||
} else { |
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 dedupe
was added in v2.2.0 so there is a chance it isn't a valid command.
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.
Ah gotcha. Great catch—we don't expect users to be on yarn 2, but to be fair we don't have any documentation / checks around that fact, so there's some work to do here. A while back Danny had asked me to make a simple utility for running yarn commands given that we have to support two versions; it's probably time for me to revisit that.
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
…ctmode-gen * 'main' of github.com:redwoodjs/redwood: Update yarn.lock fix(deps): update graphql-tools monorepo (redwoodjs#5487) v1.3.2 Update yarn.lock fix: Run dedupe during upgrade on yarn 3 (redwoodjs#5458) Fix/ts resolvers type (redwoodjs#5437) fix(deps): update dependency graphql to v16.5.0 (redwoodjs#5488) fixed typo of roll to role (redwoodjs#5484) fix(deps): update typescript-eslint monorepo to v5.23.0 (redwoodjs#5489) chore(deps): update dependency cypress to v9.6.1 (redwoodjs#5482) chore(deps): update dependency firebase to v9.8.1 (redwoodjs#5485) fix(deps): update dependency @types/aws-lambda to v8.10.97 (redwoodjs#5486) chore(deps): update dependency @nhost/nhost-js to v1.1.10 (redwoodjs#5479) chore(deps): update dependency @nhost/hasura-auth-js to v1.1.5 (redwoodjs#5478) Fixing type for BrowserOnly children (redwoodjs#5475) fix: Run dedupe during upgrade on yarn 3 (redwoodjs#5458) Fix/ts resolvers type (redwoodjs#5437)
Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com>
This PR changes
yarn rw upgrade
to runyarn dedupe
for projects on Yarn 3. Right now, the dedupe step is skipped if yarn version is higher than 1.@thedavidprice also pointed this out earlier but for some reason, I thought we already have the required changes. #4940 (comment)