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(crwa): remove yarn install preference #9859

Closed
wants to merge 1 commit into from

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jan 21, 2024

@Tobbe told me he couldn't run yarn create redwood-app with yarn v1. I was able to reproduce and so was @Josh-Walker-GM:

$ yarn create redwood-app test
yarn create v1.22.21
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 🔨  Building fresh packages...

success Installed "create-redwood-app@6.6.2" with binaries:
      - create-redwood-app
[########################################################################################] 88/88------------------------------------------------------------------
                🌲⚡️ Welcome to RedwoodJS! ⚡️🌲
------------------------------------------------------------------
✔ Compatibility checks passed
✔ Creating your Redwood app in test based on command line argument
✔ Select your preferred language · TypeScript
✔ Do you want to initialize a git repo? · no / Yes
✔ Enter a commit message · Initial commit
✔ Do you want to run yarn install? · no / Yes
✔ Project files created
✔ Installed node modules
✔ Generated types
✔ Initialized a git repo with commit message "Initial commit"

Thanks for trying out Redwood!

 ⚡️ Get up and running fast with this Quick Start guide: https://redwoodjs.com/quick-start

Fire it up! 🚀

 > cd test
 > yarn rw dev

✨  Done in 100.70s.
$ cd test
$ yarn rw dev
Internal Error: root-workspace-0b6124@workspace:.: This package doesn't seem to be present in your lockfile; run "yarn install" to update the lockfile
    ...

I'm not sure if this is new as of yarn v4 or not, but I think it's time we removed the yarn install preference in CRWA; I brought it up before in #9786 (comment) but we held off at the time.

@Tobbe I'm happy to cherry pick this into the next branch and release a patch.

@jtoar jtoar added the release:fix This PR is a fix label Jan 21, 2024
@jtoar jtoar added this to the next-release-patch milestone Jan 21, 2024
@jtoar jtoar changed the title remove yarn install preference fix(crwa): remove yarn install preference Jan 21, 2024
@@ -798,10 +630,6 @@ async function createRedwoodApp() {
`cd ${path.relative(process.cwd(), newAppDir)}`
)}`
)}`,
!yarnInstall &&
Copy link
Member

Choose a reason for hiding this comment

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

I think the logic is flipped here. We should always include the instruction to run yarn install now, right?
(Hold changing anything, I'm still reviewing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep you're right I think oversight on my part in making this too fast

@@ -76,7 +76,6 @@ describe('create-redwood-app', () => {
Fire it up! 🚀

> cd redwood-app
> yarn install
Copy link
Member

Choose a reason for hiding this comment

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

I think this change should be reverted

@Tobbe
Copy link
Member

Tobbe commented Jan 23, 2024

Closing this in favor of #9861

@Tobbe Tobbe closed this Jan 23, 2024
@jtoar jtoar removed this from the next-release-patch milestone Jan 25, 2024
@jtoar jtoar deleted the ds-crwa/remove-install-preference branch January 25, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants