Skip to content
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(cli): avoid npx during upgrade command #10479

Merged
merged 4 commits into from
Apr 19, 2024

Conversation

Josh-Walker-GM
Copy link
Collaborator

Problem
Fixes #10466.

During yarn rw upgrade we check the version of npx installed. It's entirely possible that npx is not available or installed.

We only do this check because we have to handle dedupe differently between yarn v1 and yarn >v1. We specify that redwood projects should be using yarn v4 using "packageManager": "yarn@4.1.1" in the package.json. Therefore when following the recommended setup users should not be using yarn v1 in their redwood projects.

Changes

  1. Avoid npx version check.
  2. Skip dedupe step if for some reason have yarn v1. When this happens we log a warning message to tell the user to run a command manually to dedupe.

@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Apr 19, 2024
@Josh-Walker-GM Josh-Walker-GM added this to the next-release-patch milestone Apr 19, 2024
@Josh-Walker-GM Josh-Walker-GM self-assigned this Apr 19, 2024
// Redwood projects should not be using yarn 1.x as we specify a version of yarn in the package.json
// with "packageManager": "yarn@4.1.1" or similar.
task.skip(
"Yarn 1.x doesn't support dedupe directly. Please use `npx` and run `npx yarn-deduplicate` manually.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried someone (maybe me!) will see this in a few months/years and think "Ohh, we can be more helpful here and just run npx for them!" and then go ahead and reintroduce the code you just got rid of.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could try running npx yarn-deduplicate maybe, and then, if that doesn't work we could install yarn-deduplicate locally and run it with yarn yarn-deduplicate. And if all of that doesn't work we could just tell the user to upgrade to yarn v4.

Or we skip trying to be nice and just tell them to install yarn v4 and then run yarn dedupe manually.

Either way, a comment here about npx maybe not being available on the user's system is probably a good idea (as I hinted at in my first comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated the comment to include why we do not want to do this automatically (db4b9c4)

I don't have any particular metrics to back this up but I would expect almost all redwood users to be using yarn v4 as it is specified in the package.json. People upgrading projects might not though. I agree we could try, then install, and then try again to do it automatically but I would say we should avoid that for now unless it becomes clear a number of people are running into it and it's a bad dx.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we should avoid that for

I agree. But if we can't run npx, most likely the user won't be able to either. So maybe tell them to upgrade to yarn 4 instead of suggesting something that most likely will fail? What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer even try to use npx so it could be available although now we don't require it to be.

Happy to switch the message to be upgrade yarn rather than use npx

@Josh-Walker-GM Josh-Walker-GM added the fixture-ok Override the test project fixture check label Apr 19, 2024
@Josh-Walker-GM Josh-Walker-GM merged commit ed135dc into main Apr 19, 2024
46 checks passed
@Josh-Walker-GM Josh-Walker-GM deleted the jgmw/fix-no-npx-for-upgrade branch April 19, 2024 19:17
Josh-Walker-GM added a commit that referenced this pull request Apr 19, 2024
**Problem**
Fixes #10466.

During `yarn rw upgrade` we check the version of `npx` installed. It's
entirely possible that `npx` is not available or installed.

We only do this check because we have to handle dedupe differently
between yarn v1 and yarn >v1. We specify that redwood projects should be
using yarn v4 using `"packageManager": "yarn@4.1.1"` in the
`package.json`. Therefore when following the recommended setup users
should not be using yarn v1 in their redwood projects.

**Changes**
1. Avoid `npx` version check.
2. Skip dedupe step if for some reason have yarn v1. When this happens
we log a warning message to tell the user to run a command manually to
dedupe.
Josh-Walker-GM added a commit that referenced this pull request Apr 19, 2024
**Problem**
Fixes #10466.

During `yarn rw upgrade` we check the version of `npx` installed. It's
entirely possible that `npx` is not available or installed.

We only do this check because we have to handle dedupe differently
between yarn v1 and yarn >v1. We specify that redwood projects should be
using yarn v4 using `"packageManager": "yarn@4.1.1"` in the
`package.json`. Therefore when following the recommended setup users
should not be using yarn v1 in their redwood projects.

**Changes**
1. Avoid `npx` version check.
2. Skip dedupe step if for some reason have yarn v1. When this happens
we log a warning message to tell the user to run a command manually to
dedupe.
Josh-Walker-GM added a commit that referenced this pull request Apr 19, 2024
**Problem**
Fixes #10466.

During `yarn rw upgrade` we check the version of `npx` installed. It's
entirely possible that `npx` is not available or installed.

We only do this check because we have to handle dedupe differently
between yarn v1 and yarn >v1. We specify that redwood projects should be
using yarn v4 using `"packageManager": "yarn@4.1.1"` in the
`package.json`. Therefore when following the recommended setup users
should not be using yarn v1 in their redwood projects.

**Changes**
1. Avoid `npx` version check.
2. Skip dedupe step if for some reason have yarn v1. When this happens
we log a warning message to tell the user to run a command manually to
dedupe.
dac09 added a commit that referenced this pull request Apr 22, 2024
…g-gen-mw-p2

* 'main' of github.com:redwoodjs/redwood:
  fix(deps): update React to latest canary 19.x (#10496)
  fix(upgrade): Download yarn patches (#10497)
  Revert React 19 upgrade (#10482 and #10491) (#10495)
  Modified type for describeScenario (#10468)
  fix(upgrade): Download yarn patches (#10491)
  fix(deps): update React to latest canary 19.x (#10482)
  chore(dataMigrate): Fix package.json directory value (#10490)
  RSC: css preinit: Add required option precedence (#10481)
  chore(renovate): Remove config for unused package (#10489)
  RSC: Update test projects to use new @apollo/client-react-streaming (#10488)
  RSC: Remove unused vite patch from test-project-rsa (#10487)
  chore(ssr): Switch to use `@apollo/client-react-streaming` package (#10484)
  chore(docs): Move .mdx import into separate file (#10486)
  fix(cli): avoid `npx` during upgrade command (#10479)
  fix: Fixes Unknown Fragment issues due to GraphQL Tag type mismatch in web (#10357)
  RSC: Clean up some comments and logs (#10480)
  feat: redwoodjs/auth-dbauth-middleware - Part 2/3 - Auth Middleware for dbAuth to authenticate users via cookie (#10447)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixture-ok Override the test project fixture check release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug?]: Updating from 7.3.0 to 7.4.1 will fail, if there is no npx available
2 participants