[CRWA]: Switch to using enquirer, add engine compatibility override option #6723
Conversation
There was a problem hiding this comment.
Thanks for getting started on this @ehowey! I know this is still a draft but I left a few comments. And to answer your ending question, I don't think this PR has to wait for the other prompts to be moved over. So it can go in when it's ready!
Seperate Listr to avoid listr2/listr2#296
Having it all in one Listr seems ideal to me from a visual standpoint. I'll try to understand this issue a bit more. Output spills over is it?
|
And @ehowey if you'd like me to resolve merge conflicts more than happy to, but would require that I rebase and force push. Just let me know—it's just a version conflict for yargs. |
|
Thanks for the great code review! I'll take a look and update this soon - maybe later this evening. I should be fine with the git merge, I just got lost in a messed up git reset last time! EDIT: I made some updates but gotta do some testing on it still. More on the issue with multiple Listrs and terminal height though:
So the short answer is yes - if the content you output via Listr overflows the terminal height it gets cutoff and disappears. This doesn't happen with a console log. I often use a pretty small terminal window directly in VSCode and noticed this error happening right away. I tried a few work arounds, probably the easiest workaround is to really shorten the information we give to the user about the engine error, max of maybe two lines. I think how NextJS gets around this is that they have a docs page for every error code. This lets you put way more detail on the actual docs page and keep your errors very concise. I have found that pattern pretty helpful a few times with NextJS because of how hyper specific the error docs were. For example you might output something like: |
ehowey
changed the title
|
Alright folks - I did some testing and caught a bit of an odd bug with Listr and Enquirer not playing nice together but it isn't blocking just leads to some ugly code - see the comments labelled TODO in the actual code for the section I mean. One thing to note when someone selects JS as their language using the prompt the I ran through the following scenarios (M1 Macbook Air, VS Code terminal): Node 18
Node 16
Screenshots:
|
There was a problem hiding this comment.
@ehowey thanks for sticking with us! This is probably my last review. As in, I think after all these my comments are resolved, this is good to merge. 🚀 Thanks again—this is a great feature for us, and will go a long way towards communicating to users that Redwood is likely compatible with more Node.js targets than meets the eye.
After it's merged I'll run it by David to get more feedback on the visual aspect of it, but don't want to bikeshed on it too much since functionally it works and this has been open for a while already.
|
Thanks @jtoar those were some great suggestions and visual improvements. I've made those changes and did some visual testing, everything looked good in my terminal. It has been really fun to collaborate with your team on this PR; great experience for me and nice to learn a bunch about building this kind of CLI tool in the process. I think this one is ready to go! Next maybe we give an option for git init? That would fix my two personal annoyances with the install process! |
There was a problem hiding this comment.
That's great to hear! Sometimes I can't tell if I'm nitpicking too much during code reviews, but that is kind of the point at the same time. 😅 But that's all to say, I'm glad we're making it worth your while!
Oh yeah, I'm all for a git init option or prompt. Would happily accept that one. 🙏
…ption (#6723) * Add engine check and prompt * Language tweak * Add link and additional text to error * Migrate to Listr2 * WIP * Complete engine checks * Typo, concurrent, simpler output * Fix broken quit process, add quit install message * Remove ToDo, update code comments * Make suggested visual changes * Collapse Listr
…aching * 'main' of github.com:redwoodjs/redwood: (21 commits) [Tutorial]: Fix Typescript code blocks inconsistency (#6801) chore: update all contributors Custom auth: Fix comment in template (#6804) fix(deps): update dependency eslint to v8.26.0 (#6785) [CRWA]: Switch to using enquirer, add engine compatibility override option (#6723) (docs): Minor Command update about Storybook (#6722) docs: Add mocking useLocation to docs (#6791) Update generated render.yaml (#6771) fix flightcontrol config template (#6789) fix: publish canary using premajor (#6794) Strip resetToken and resetTokenExpiresAt from dbAuth forgotPassword handler (#6778) Fix WebAuthn when event body is base64 encoded (like when deploying to Vercel) (#6757) fix(deps): update jest monorepo (#6787) fix(deps): update dependency react-hook-form to v7.39.1 (#6786) fix(deps): update dependency fastify to v4.9.2 (#6781) fix(deps): update dependency @apollo/client to v3.7.1 (#6780) chore: fix and rebuild test project fixture (#6775) fix: add prisma resolutions to tutorial e2e test proj (#6772) fix(deps): update prisma monorepo to v4.5.0 (#6485) Fix dbauth webauthn template (redundant type import) (#6769) ...
…ption (#6723) * Add engine check and prompt * Language tweak * Add link and additional text to error * Migrate to Listr2 * WIP * Complete engine checks * Typo, concurrent, simpler output * Fix broken quit process, add quit install message * Remove ToDo, update code comments * Make suggested visual changes * Collapse Listr
Hey All,
This should be considered a draft, I want to do some more testing. But I drafted what CRWA could look like with a switch over to enquirer. I think this sets a basis for future improvements really nicely.
One area I ran into issues was with the engine checks. I was hoping to only have one Listr, however had to keep it split out into it's own seperate Listr process because of the bug with overflowing terminal height and text display (see listr2/listr2#296).
Once @jtoar moves other areas of the codebase over to enquirer I can use this as a basis for the main PR (#6380) . But maybe we don't need to wait?