(feat): Add prompt for overriding engine error in CRWA

[ehowey]

Related to #3989

This adds a prompt to continue through any engine errors/warnings in the install process. It also moves the engine checks to the very start of the install process.

Because of how ListR renders I had to make the decision to move the engine check higher up in the process in its own task separate from the main install tasks. It wasn't possible to insert a prompt in the middle of ListR tasks, at least none of the ways I tried would actually render properly in the terminal. I actually like having the engine checks at the start - if the tests pass I show a nice green check mark and all continues as normal. However if the test fails then you get an error message and a prompt asking you whether you want to continue with the install or not.

Some questions for discussion:

• Do you like it at the start? Should it go somewhere else in the flow of the install process?
• Would you prefer it to be simply a confirmation prompt instead of a select list. For example we could show a message that was more like "Override errors and continue install? Y/N". This is how I originally built it but it felt too quick and easy to just hit enter, and we probably want people to pause for a moment to consider what they want to do if they get an error.
• Any suggestions for language/wording for any of the user messages and prompts. For example if the user chooses to quit the install do we want to show a longer or more detailed message? Give them the option to open their browser window right to the proper docs page?
• I am used to frontend react code, e.g. useState so if there are code suggestions/format stuff feel free to shout them out. What I have here works, but maybe has some code smells?

[Tobbe]

• Do you like it at the start? Should it go somewhere else in the flow of the install process?

I like it at the start. I feel that anything that's a potential blocker should be at the start. It makes no sense to answer a bunch of questions first, just to realize you can't install anyway 🙂

[ehowey]

Just checking in with this one and whether you want to work through it to get it merged in the next week or if there are some bigger conversations happening about CRWA between core members?

[Tobbe]

Thanks for the ping @ehowey. I'm sorry I didn't get to this today. It does look good, I just want to take it for a spin locally. And then I'll talk to @thedavidprice about that documentation we wanted to connect to this

[ehowey]

Okay sounds good - maybe also start needing to bike shed the possibility to skip these checks in a CI env.

[Tobbe]

So this is what I see when I try it (just for reference for everyone else)

In addition to the red text you already have there, how about adding (also in red)
"this might make your Redwood project incompatible with some deploy targets" or something along those lines

I like the orange link, but it would be even better if it was a clickable link, do you think you can change that? (we use the terminal-link package for this in other parts of the code base). You can link here for now: https://redwoodjs.com/docs/tutorial/chapter1/prerequisites

[ehowey]

I'm going to be stalled out on making that change for a few days Tobbe - sorry! I should be able to get to it early next week.

[ehowey]

Sorry for the delay here Tobbe - I completely blanked on what week it was and had another commitment! September is flying by.

Here is what it looks like now:

[Tobbe]

Sorry for the delay here Tobbe

No need to be sorry. Take the time you need 🙂

[Josh-Walker-GM]

Hey we recently switched the CRWA from listr to listr2. This might make things a little easier here give that listr2 has nice support for prompting (listr2 user input docs). Prompting from within tasks is now easy so you won't be forced to, although you still can if you want, prompt before listr runs.

Also don't worry I've found the changes needed for the listr to listr2 update to be minimal.

[replay-io]

14 replays were recorded for f98c7ac.

0 Failed

14 Passed

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

View test run on Replay ↗︎

[ehowey]

Just a note to say I merged those changes and did the minor migration to using the rendererOptions object, otherwise like Josh said it just worked the same as before.

In terms of Listr2 and prompting the user, that is good news. But probably a bigger conversation for the core team because of the required switch from prompts to enquirer. Thanks for those updates and migration Josh, it looks like the Listr2 package has got some good benefits with the same API surface more or less.

[jtoar]

@ehowey enquirer is a lot smaller and faster and is what Nx uses, so we're down to switch it out. It's used in quite a few tooling-related parts of the codebase (e.g. the release script) so I'll go ahead and make the switch!

[ehowey]

Are you just going to push against this PR or a separate PR for the switch to enquirer and then I can update this PR afterwards?

[jtoar]

@ehowey separate PR! And I can update your changes here for you unless you'd like to handle it? (I just feel bad introducing merge conflicts, those are never fun.)

[ehowey]

I don't mind handling it. It might open the opportunity for a mini-refactor as enquirer + Listr2 allows for prompts to be handled within Listr itself versus separated out as I have them now. I'm thinking that this will be helpful for any future prompts the team wants to add and a better code pattern long term.

[ehowey]

Closing in favor of #6723