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

Enable yarn 3 for Redwood projects #4444

Merged
merged 48 commits into from
Mar 3, 2022
Merged

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Feb 10, 2022

Update: I'm now focused on CI, the contributing flow, and making sure that everything's still backwards compatible.

This PR enables yarn 3 for Redwood projects. Right now, the goal is to do as little as possible (i.e., no breaking changes) to make this happen. So by yarn 3's standards, this is super hacky / all workarounds / very limited in scope (we don't plan on supporting just any install strategy yet).

Anyway, the goal is that users with existing Redwood projects can do something simple like this to upgrade:

yarn set version stable
# Users can't have just any `.yarnrc.yml` settings,
# so we need to give them a template to copy or a codemod to write them:
npx @redwoodjs/codemods upgrade-yarn

# This has to be last--after tweaking `.yarnrc.yml` and `.gitignore`.
yarn install

Note
Until we publish this PR live, use this command:
npx @redwoodjs/[email protected] upgrade-yarn

Problem

Just to recap why we're doing this, yarn 3 has a lot more rules than yarn 1, one of them being that you can't run just any bin anymore. It has to be a bin of one of your top-level dependencies.

The way the RedwoodJS CLI works is that it's a bin of @redwoodjs/cli. It gets into projects via @redwoodjs/core, so it's a dependency of a dependency. It's not top-level.

That means after upgrading to yarn 3, you can't do this:

dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn rw dev
Usage Error: Couldn't find a script named "rw".

$ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...

A visual way of seeing why is by running yarn why:

dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn why @redwoodjs/cli
└─ @redwoodjs/core@npm:0.44.1
   └─ @redwoodjs/cli@npm:0.44.1 (via npm:v0.44.1)
dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn why @redwoodjs/core
└─ root-workspace-0b6124@workspace:.
   └─ @redwoodjs/core@npm:0.44.1 (via npm:v0.44.1)

@redwoodjs/cli isn't a dependency of the root workspace, it's a dependency of @redwoodjs/core, and that makes all the difference now.

Another debugging tool is yarn bin. If you can run a bin, yarn bin lists it. Right now yarn bin lists nothing:

dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn bin
➤ YN0000: Done in 0s 1ms

An easy way to solve this is to make @redwoodjs/cli a top-level dependency. Then you can run yarn rw:

# Note: `yarn add -W` isn't a thing anymore and this does indeed have consequences for the setup command I'll get to later
dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn add -D @redwoodjs/cli
dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn why @redwoodjs/cli
├─ @redwoodjs/core@npm:0.44.1
│  └─ @redwoodjs/cli@npm:0.44.1 (via npm:v0.44.1)
│
└─ root-workspace-0b6124@workspace:.
   └─ @redwoodjs/cli@npm:0.44.1 (via npm:^0.44.1)
dom@evaM1 ~/p/r/_/y/yarn3 (main)> yarn bin
➤ YN0000: redwood
➤ YN0000: rw
➤ YN0000: rwfw
➤ YN0000: Done in 0s 26ms

But as @thedavidprice said, will it dev? No, because a lot of our CLI commands are really just execa('yarn run ...') statements, which brings us back to square one:

web | Usage Error: Couldn't find a script named "cross-env".
Usage Error: Couldn't find a script named "prisma".
gen | Usage Error: Couldn't find a script named "rw-gen-watch".

Strategy

The strategy this PR uses is the one suggested by @larixer in the original issue: proxy bin invocations by listing them as @redwoodjs/core bins but really just requiring them from their true package location behind the scenes.

An alternative to this is doing what we already do for the prisma command: just hardcode the path to the bin:

execa.sync(
`"${path.join(rwjsPaths.base, 'node_modules/.bin/prisma')}"`,
args,
{
shell: true,
cwd: rwjsPaths.base,
stdio: 'inherit',
cleanup: true,
}
)

yarn 3 advises against doing this because it ties you to an install strategy (node_modules is just an implementation detail and may not exist at all depending on the install strategy). But we're ok with tying ourselves to an install strategy for now.

Contributing

Half the reason this PR has been difficult is that the normal contributing flow doesn't apply. There's a temporary (i.e. I plan to remove it before merging) commit that adds a new contributing command yarn project:yarn3 to enable trying out this PR locally: f0c0b96 (#4444).

The script assumes you have a redwood project on yarn 3. I.e. you set your project to yarn 3 via yarn set version stable and copied the .yarnrc.yml from the RedwoodJS framework to your project's and yarn install.

Note that since Redwood projects technically are using yarn 3 after running these steps (even though the CLI doesn't work), I tried yarn patch and the portal protocol as contributing workflows, but yarn patch doesn't work for bins since patches are resolved after bins, and portal doesn't seem to 100% work 'cause we're still on the node_modules linker.

Feedback

@merceyz, @larixer we welcome any feedback! We know that we'll have to refactor a lot of the CLI in the future to adhere to the new best practices (we may just distribute it as a separate binary and not run it via yarn run at all which may make things easier too), but for now we're doing what we can with what we have.

CLI Commands Checklist

  • build
  • check
  • console
  • data-migrate
  • deploy
    • everything works but serverless. I haven't confirmed it'll break but it looks like it will
  • destroy
  • dev
  • exec
  • generate
  • info
  • lint
  • open
  • prerender
  • prisma
  • record
  • serve (depends on build)
  • setup
  • storybook
  • test
  • ts-to-js
  • type-check
  • upgrade

@jtoar jtoar added topic/crwa create-redwood-app v1/for-consideration release:feature This PR introduces a new feature labels Feb 10, 2022
@jtoar jtoar assigned dac09 and jtoar Feb 10, 2022
@jtoar
Copy link
Contributor Author

jtoar commented Feb 10, 2022

Note that I'm not sure if we should add yarn to the create-redwood-app template or not. We do want them to be on a specific version with specific settings (the settings are critical), but maybe we can enforce that without having to commit yarn to the repo.

On the other hand, you do commit yarn now (we do it in the framework), so why not just do it from the start?

Also I wanted to use corepack, but we still support node 14 so it's not viable yet.

@jtoar
Copy link
Contributor Author

jtoar commented Feb 11, 2022

Update: I've made sure to handle this.

Another thing that's gonna be tricky is making sure users can still run the CLI from the web and api directories.

@thedavidprice
Copy link
Contributor

@jtoar I'm happy with the pattern you're using here. It's understandable and manageable. I am very curious about feedback from the Yarn team, however. One step forward...

Q1

Note that I'm not sure if we should add yarn to the create-redwood-app template or not. We do want them to be on a specific version with specific settings (the settings are critical), but maybe we can enforce that without having to commit yarn to the repo.

^^ Oh, this gives me an idea — what if we set --yarn3 as an installation command option? It would go through the steps to init config. And, therefore, we'd have a path to go from "experimental" to final; i.e. set the default to "false" at first until we can verify, then flip the switch to "true".

Thoughts?

Q2

Another thing that's gonna be tricky is making sure users can still run the CLI from the web and api directories.

^^ Cause the CLI package will now need to be included in api/package.json and web/package.json. That's ok and expected.

Q3

Also I wanted to use corepack, but we still support node 14 so it's not viable yet.

If it makes this work, I'm all for bumping the Node requirements to v16 for 1.0.0 GA. That's not unreasonable (or hard). We're asking devs to update development environment only — deploy targets will still be v14.

Q4

An easy way to solve this is to make @redwoodjs/cli a top-level dependency.

To confirm, you are still planning to decouple CLI from Core, correct? If anything, we should rename Core --> Webpack and then include Webpack as a dependency in CLI.

TBD with Peter and Danny when the time is right.

@jtoar jtoar force-pushed the ds-upgrade-projects-to-yarn-3 branch from aab3b8d to 0716ebc Compare February 19, 2022 11:00
@merceyz
Copy link
Contributor

merceyz commented Feb 19, 2022

If it makes this work, I'm all for bumping the Node requirements to v16 for 1.0.0 GA. That's not unreasonable (or hard). We're asking devs to update development environment only — deploy targets will still be v14.

Corepack has been backported to v14 so you'd only need to bump it to v14.19.0 if that's the only requirement
https://github.com/nodejs/node/blob/dde2f788877ac77329cc3bcc309dc346aa385550/doc/changelogs/CHANGELOG_V14.md#2022-02-01-version-14190-fermium-lts-richardlau

jtoar pushed a commit that referenced this pull request Feb 21, 2022
we're not using zero installs, and this isn't CI; see #4444 (comment)
@jtoar jtoar linked an issue Feb 21, 2022 that may be closed by this pull request
@jtoar jtoar marked this pull request as ready for review February 22, 2022 19:53
jtoar pushed a commit that referenced this pull request Feb 22, 2022
we're not using zero installs, and this isn't CI; see #4444 (comment)
@jtoar jtoar force-pushed the ds-upgrade-projects-to-yarn-3 branch from 69f186d to 571113a Compare February 22, 2022 19:53
@thedavidprice thedavidprice mentioned this pull request Feb 22, 2022
21 tasks
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

@jtoar Phenomenal work here, Dom. I had one question about setup ui chakra-ui, which I did test myself with Yarn 3 and setup completed successfully.

Let's hold merge until we discuss timing. Stoked to see this go in!

const tscForAllSides = sides.map((side) => {
const projectDir = path.join(getPaths().base, side)
// -s flag to suppress error output from yarn. For example yarn doc link on non-zero status.
// Since it'll be printed anyways after the whole execution.
return {
cwd: projectDir,
command: `yarn -s tsc --noEmit --skipLibCheck`,
command: `yarn ${
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtoar what if we created a helper for running yarn commands so all the logic is in one place? Something like runYarnCommand

dac09
dac09 previously requested changes Mar 1, 2022
Copy link
Contributor

@dac09 dac09 left a comment

Choose a reason for hiding this comment

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

This is such tremendous work @jtoar - I applaud your patience and thoroughness here!

Would you mind ✋ HODLING the PR - some of the minor comments are addressed/dismissed?

@thedavidprice
Copy link
Contributor

thedavidprice commented Mar 2, 2022

@jtoar I've tested rwfw against this branch with a my project running both Yarn 1 and Yarn 3.

Yarn 3 ✅

Yarn 1

  • using project:dep (then yarn install) and project:copy separately ✅
  • using project:sync ⛔️
$ /Users/price/Repos/tdp-redwood-tutorial-test/node_modules/.bin/rw help
/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/internal/dist/paths.js:72
    throw new Error(`Could not find a "${CONFIG_FILE_NAME}" file, are you sure you're in a Redwood project?`);
    ^

Error: Could not find a "redwood.toml" file, are you sure you're in a Redwood project?
    at getConfigPath (/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/internal/dist/paths.js:72:11)
    at getBaseDir (/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/internal/dist/paths.js:84:34)
    at getPaths (/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/internal/dist/paths.js:120:30)
    at Object.<anonymous> (/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/telemetry/dist/telemetry.js:21:41)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Strange that it's a telemetry error, but I think that might be a herring. I tried REDWOOD_DISABLE_TELEMETRY=1 and got the same error.

@jtoar
Copy link
Contributor Author

jtoar commented Mar 2, 2022

@thedavidprice could you give me a little more info on how yarn rwfw project:sync failed? Did it fail right away? Or did a subsequent yarn rw command fail after using yarn rwfw project:sync? The error in your comment says you tried to run yarn rw help which is why I'm asking.

@thedavidprice
Copy link
Contributor

It failed on all the commands that was just the last one I tried. The failure was immediate.

@cloudcompute
Copy link

Before you implement yarn 3 in Redwood, I request you to go through this

@thedavidprice
Copy link
Contributor

Hi @Ramandhingra Please see my reply on your forum thread.

tl;dr:
When we merge this PR, we think there’s a great chance you can use pNpm with your Redwood Project.

Thanks!

@dac09 dac09 dismissed their stale review March 3, 2022 08:48

Discussed realtime

…rojects-to-yarn-3

* 'main' of github.com:redwoodjs/redwood: (29 commits)
  Update dependency eslint-config-prettier to v8.5.0 (#4631)
  Update dependency msw to v0.38.2 (#4630)
  Update dependency @clerk/types to v1.27.0 (#4628)
  Update dependency @clerk/clerk-js to v2.16.0 (#4627)
  Update dependency @types/react-dom to v17.0.13 (#4629)
  Update dependency @types/testing-library__jest-dom to v5.14.3 (#4624)
  Update dependency @types/aws-lambda to v8.10.93 (#4620)
  Webhook verifiers: Make them all support timestamp diff check (#4608)
  Add custom log payload support to logFormatter (#4619)
  Update dependency @types/react-dom to v17.0.12 (#4621)
  part II of #4623 (#4626)
  Update dependency systeminformation to v5.11.6 (#4611)
  Update dependency @clerk/clerk-js to v2.15.0 (#4606)
  remove Redwood extension from vscode rec (#4613)
  (fixture chore) pin fixture autoprefixer 9.8.8 (#4623)
  update yarn.lock
  v0.47.1
  update yarn.lock
  fixi(prisma): Set default cwd for runCommand task to base (#4604)
  Update actions/checkout action to v3 (#4610)
  ...
@jtoar jtoar enabled auto-merge (squash) March 3, 2022 09:11
@jtoar jtoar merged commit ebfce3d into main Mar 3, 2022
@jtoar jtoar deleted the ds-upgrade-projects-to-yarn-3 branch March 3, 2022 09:14
@jtoar jtoar added this to the next-release milestone Mar 3, 2022
dac09 added a commit that referenced this pull request Mar 3, 2022
…ors-db-auth

* 'main' of github.com:redwoodjs/redwood:
  [WIP] Enable yarn 3 for Redwood projects (#4444)
  Update dependency eslint-config-prettier to v8.5.0 (#4631)
  Update dependency msw to v0.38.2 (#4630)
  Update dependency @clerk/types to v1.27.0 (#4628)
  Update dependency @clerk/clerk-js to v2.16.0 (#4627)
  Update dependency @types/react-dom to v17.0.13 (#4629)
  Update dependency @types/testing-library__jest-dom to v5.14.3 (#4624)
  Update dependency @types/aws-lambda to v8.10.93 (#4620)
  Webhook verifiers: Make them all support timestamp diff check (#4608)
  Add custom log payload support to logFormatter (#4619)
@thedavidprice thedavidprice changed the title [WIP] Enable yarn 3 for Redwood projects Enable yarn 3 for Redwood projects Mar 3, 2022
@thedavidprice thedavidprice modified the milestones: next-release, v0.48.0 Mar 4, 2022
@stephenhandley
Copy link
Contributor

stephenhandley commented Apr 1, 2022

I've run through the 5 steps for upgrading to yarn 3 here and still seeing

$ yarn redwood upgrade --tag rc       
Usage Error: Couldn't find a script named "redwood".

$ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...

Ended up resolving by going back to yarn v1

yarn set version classic

@jtoar
Copy link
Contributor Author

jtoar commented Apr 2, 2022

Hey @stephenhandley, have you tried using the codemod?

npx @redwoodjs/codemods@canary upgrade-yarn

The codemod uses corepack which handles some edge cases.

I just read your comment again; maybe I should clarified this first: is it just that the upgrade command with the rc tag doesn't work on yarn 3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature topic/crwa create-redwood-app
Projects
No open projects
Status: Archived
Status: Done
Development

Successfully merging this pull request may close these issues.

POC: Migrating a Redwood Project to Yarn v3
7 participants