[CLI]: Upgrade from Listr to Listr2

[Josh-Walker-GM]

Problem
The current listr CLI library has poor support for user input. This prevents prompts from working correctly given the prompt text is drawn over by the various tasks text. This restricts our ability to use prompts to interact with the user who runs commands.

Solution
To update to use a different CLI library. Easiest and quickest way forward with this is to update to listr2 a very popular alternative to listr which includes good support for user input via the, again very popular, enquirer package. This PR upgrades the CLI to use listr2.

Problems that remain
listr2 has not yet updated to fix listr2/listr2#296. The beta version does address this issue however the update has not yet been pushed to the main version. I don't see this as a deal breaker given we currently have to live with the existing issue anyway as listr has the same issue.

Changes

1. Added listr2 and removed the listr and the associated list-verbose-renderer which is no longer needed.
2. Updated package imports from listr to listr2 where necessary.
3. Updated collapse:false to be within the required rendererOptions config option.
4. Added rendererOptions: {collapse: false} in a few places it was previously not included

Outstanding work

• I am aware discussions around CLI improvements are ongoing and it may be the case the CLI moves away from listr or listr2 in the future. Given the apparent ease of this listr to listr2 transition I felt comfortable enough making this PR anyway. Project leads will need to consider this change appropriate.
• Currently the tests fail. I believe this is due to the mocking of the listr package. I am not experienced with testing and would require assistance confirming this is the case and with updating the tests as required.
• I have tested numerous CLI commands with this update and they appear to work well - indeed they should work identically. We will need to further test to ensure all the commands function well and look correct.
• Some may be concerned about the full scale switch out of one package for another so abruptly. I think given the existing structure of the code around listr this transition is made much easier and is appropriate.

Updates

1. The number of tests failing has been dramatically reduced with the changes listed below. Provided these changes are correct then I do think the only remaining issue is related to the mocking. All tests pass, just need someone with experience to confirm the changes are valid. A CI test related to package versions fails but again I'm not familiar with this particular test.
   1. t.setRenderer("silent") changed to t.options.renderer = "silent"
   2. t._tasks changed to t.tasks
   3. Updated jest.mock in createYargsForComponentGeneration.test.js
   4. Removed the manual mocking implementation in src/commands/__tests__/build.test.js and used the automatic one instead.

[Josh-Walker-GM]

@Tobbe, @cannikin, @peterp and @thedavidprice may be specifically interested given their CLI experience. Apologies to you all for any repeated notifications recently and sorry to anyone not mentioned.

[Tobbe]

@jtoar pinging you too, because I know you're interested in the cli

[Tobbe]

To fix this you need to run yarn build:test-project --rebuild-fixture at the root of the redwood repo

[Josh-Walker-GM]

To fix this you need to run yarn build:test-project --rebuild-fixture at the root of the redwood repo

I had tried this but didn't push anything because the only meaningful change it made was to reduce the package requirements in web. Changed @redwood/... from 3.0.2 to 3.0.1. was going to seek clarification since that seemed like a backwards step? Hadn't updated from the main branch, problem fixed.

Why the other CI tests are failing I'm unsure. The error messages aren't that illuminating to me.

[Tobbe]

Sounds like committing an updated lockfile should fix it

[Josh-Walker-GM]

Sounds like committing an updated lockfile should fix it

Ahh I didn't realise some of the test logs were expandable 🤦 Woops at forgetting it in the first place.

[Josh-Walker-GM]

Can I just confirm that the merge conflicts I fixed were done properly. I had issues like shown below and I just accepted the one in this branch rather than the main. These seemingly random number suffixes are important?

<<<<<<< listr-to-listr2
            email: 'String9868660',
=======
            email: 'String7848705',
>>>>>>> main

[Tobbe]

No, they're not important.

Thanks for asking. Reaffirms my belief we should try to get rid of that.

[Josh-Walker-GM]

The only failing test doesn't appear to have failed for any reason related to these changes.

[Tobbe]

Status update: We haven't forgotten about this one. Just waiting for the right time to merge this. It touches a lot of files, so want to limit the disturbance of other PRs. Don't worry though, we'll help you get it in if needed 🙂

[Tobbe]

@Josh-Walker-GM You've left a bunch of TODOs about supporting --verbose. I think that's a good idea to do. Can you add that? The core team has a meeting later today. I'll try to get a go-ahead on merging this now 🙂

[Josh-Walker-GM]

Okay I should mention that all the existing support for the verbose flag is maintained. The TODOs were simply extra so might correspond to commands which don't need them.

I'll do a quick pass through and add or remove those TODOs. I'll also do a general preparation to get this more up to date with main.

[Tobbe]

Yeah. Totally up to you to either implement the extra verbose support or jus remove the TODOs. The extra verbose support can come in a separate PR if you want. You decide 🙂

[Tobbe]

I have no idea what's going on here, but I can reproduce locally

Locally I do (in the RW framework)
yarn build:test-project ~/some/path

And then I go to ~/some/path and run yarn rw test api

Just noticed that this other (unrelated) PR had the same issue #6494

wmhas seems to come from here https://github.com/zloirock/core-js/blob/0400deaa99b304f5a772f5c7cc7c622e10aa47f6/packages/core-js/internals/internal-state.js#L32

[Josh-Walker-GM]

Brand new issue raised with core-js around this which may be worth watching zloirock/core-js#1133

[replay-io]

14 replays were recorded for cbf9356.

0 Failed

14 Passed

• Loads Cell mocks when Cell is nested in another story
• RBAC: Admin user should be able to delete contacts
• Loads Cell Stories
• RBAC: Should not be able to delete contact as non-admin user
• useAuth hook, auth redirects checks
• requireAuth graphql checks
• useAuth hook, auth redirects checks
• RBAC: Should not be able to delete contact as non-admin user
• RBAC: Should not be able to delete contact as non-admin user
• RBAC: Should not be able to delete contact as non-admin user
• Smoke test with dev server
• Smoke test with rw serve
• Mocks current user, and updates UI while dev server is running
• Loads MDX Stories

View test run on Replay ↗︎

[jtoar]

@Josh-Walker-GM thanks for the link, that one's been resolved!

[Josh-Walker-GM]

We appear to have all green on the CI 🎉 Are we looking to get this in now or are hanging on for some more PRs before this one? There are one or two PR's I've spotted which might need updating if this one goes in first.

[Tobbe]

@Josh-Walker-GM I'll try to get this one in asap

[Josh-Walker-GM]

@Tobbe the router "overlapping act() calls" strikes again - this isn't an issue here though am I right?

[jtoar]

@Josh-Walker-GM nah it's just something we've been seeing on and off for a while that we've tried to fix on and off for a while

[Tobbe]

I was able to reproduce the "overlapping act() calls" locally by tweaking the timings in the router tests. I gave everything a little more wiggle room in this PR #6493 so hopefully it won't complain as much moving forward.