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

Add @redwoodjs/auth package to CLI upgrade command #556

Merged
merged 3 commits into from
May 18, 2020

Conversation

thedavidprice
Copy link
Contributor

closes #546

@peterp Due to inconsistent ways Yarn itself handles 1) workspaces and 2) tags (e.g. @canary), this one got a bit messy. It is working now:

  • for each option, upgrade and @canary upgrade, it checks to see if Auth package is installed to determine if it should be included. yarn upgrade-interactive ... will do this by default. For yarn upgrade ...@canary, things were a bit messier. (Note: upgrade-interactive does not support tags)
  • refuses to --dry-run with --canary options; canary effectively force updates to most recent canary, and the yarn outdated ... doesn’t know how to handle tags

The only annoying thing left to fixe is Listr duplicating the titles when I try to dynamically change them. This was working fine until I added the logic in the first step, 'Checking installed packages...'. Any thoughts about how to make the first two duplicates correctly go away? This used to happen on create-redwood-app.js and somehow you fixed it:
Screen Shot 2020-05-17 at 10 00 49 PM

@thedavidprice thedavidprice requested a review from peterp May 18, 2020 05:26
@peterp
Copy link
Contributor

peterp commented May 18, 2020

I think you could use something like this to determine which @redwoodjs packages are already installed:

yarn list --pattern @redwoodjs --json

@thedavidprice
Copy link
Contributor Author

@peterp Thanks! And, yeah, I looked into that as well. Since this is only an issue when targeting @canary (e.g. using tags), I found it easier with the grep to get it out the door. But this doesn't support all edge case uses of --canary. I put a TODO comment in code about how to improve in the future.

The biggest annoyance was Listr title duplication. I worked around that by nesting the tasks.

All the logic is working well now. Will merge PR unless things seem missing or broken.

@thedavidprice thedavidprice merged commit 1b26562 into master May 18, 2020
@thedavidprice thedavidprice deleted the dsp-add-auth-package-CLI-upgrade branch May 18, 2020 20:57
@thedavidprice thedavidprice added this to the next release milestone May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for redwoodjs/auth to yarn rw upgrade
2 participants