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

(feat): Add TS prompt to Create Redwood App #6360

Merged
merged 5 commits into from
Sep 12, 2022

Conversation

ehowey
Copy link
Contributor

@ehowey ehowey commented Sep 8, 2022

Addresses #6275

Continued from #6332 - I got lost in git reset hell and just started fresh!

Based on the conversation from the previous PR this PR is reduced in scope and achieves the following:

yarn create redwood-app ~/tmp/rw-test388 -> Prompt for TS or JS
yarn create redwood-app ~/tmp/rw-test388 --ts -> No prompt. TS project
yarn create redwood-app ~/tmp/rw-test388 --no-ts -> No prompt. JS project (or --ts=false, which yargs also gives you for free)

A couple of notes:

  • The default for ts in Yargs is set to null so that the user is prompted by default, default for the prompt is Y though.
  • I added an error for when --no-ts --no-yarn-install are used together
  • Please feel free to change/modify the welcome message. It is a simple placeholder.
  • The userArgs object is a bit of a workaround, but the best way I could think of handling the prompt override based on user input to the command line. If typescript is null, then the userArgs ends up being an empty object, which means that the prompt will be shown to the user.
  • Sorry it is such a messy diff, because of the async function for the prompt everything ends up with changes to spacing leading to a really hard to view file diff.

@Tobbe
Copy link
Member

Tobbe commented Sep 8, 2022

  • Sorry it is such a messy diff, because of the async function for the prompt everything ends up with changes to spacing leading to a really hard to view file diff.

image

"Hide whitespace" makes it super easy to read 🙂

@Tobbe Tobbe assigned Tobbe and unassigned simoncrypta Sep 8, 2022
@Tobbe Tobbe added the release:feature This PR introduces a new feature label Sep 8, 2022
@ehowey
Copy link
Contributor Author

ehowey commented Sep 9, 2022

Good feedback - made the requested changes and have done a final test run. From what I can tell it is all working as expected. Thanks for all of the support and guidance on this one Tobbe! Really appreciated!

Conversation for another place/thread but I am also going to throw up another "experimental" PR with some of those other ideas we have chatted about just to continue spitballing ideas for improvements to the install experience.

@Tobbe Tobbe merged commit 5ff58a2 into redwoodjs:main Sep 12, 2022
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Sep 12, 2022
@jtoar
Copy link
Contributor

jtoar commented Sep 12, 2022

Excited for this! Now that it's merged I just tried to try it out via yarn create redwood-app@canary test and it failed with:

/private/var/folders/dt/yks4v5m53k114qxgz6jh4pgw0000gn/T/xfs-9f7c3693/dlx-8848/.pnp.cjs:19377
      Error.captureStackTrace(firstError);
            ^

Error: create-redwood-app tried to access prompts, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: prompts
Required by: create-redwood-app@npm:2.2.5-canary.313 (via /Users/dom/.yarn/berry/cache/create-redwood-app-npm-2.2.5-canary.313-d72727c361-8.zip/node_modules/create-redwood-app/dist/)

Require stack:
- /Users/dom/.yarn/berry/cache/create-redwood-app-npm-2.2.5-canary.313-d72727c361-8.zip/node_modules/create-redwood-app/dist/create-redwood-app.js
    at Function.require$$0.Module._resolveFilename (/private/var/folders/dt/yks4v5m53k114qxgz6jh4pgw0000gn/T/xfs-9f7c3693/dlx-8848/.pnp.cjs:19377:13)
    at Function.require$$0.Module._load (/private/var/folders/dt/yks4v5m53k114qxgz6jh4pgw0000gn/T/xfs-9f7c3693/dlx-8848/.pnp.cjs:19231:42)
    at Module.require (node:internal/modules/cjs/loader:1028:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/Users/dom/.yarn/berry/cache/create-redwood-app-npm-2.2.5-canary.313-d72727c361-8.zip/node_modules/create-redwood-app/dist/create-redwood-app.js:41:39)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Object.require$$0.Module._extensions..js (/private/var/folders/dt/yks4v5m53k114qxgz6jh4pgw0000gn/T/xfs-9f7c3693/dlx-8848/.pnp.cjs:19421:33)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.require$$0.Module._load (/private/var/folders/dt/yks4v5m53k114qxgz6jh4pgw0000gn/T/xfs-9f7c3693/dlx-8848/.pnp.cjs:19244:22)

Seems like a simple fix: just add prompts as a dependency to create-redwood-app since it imports prompts. Also my global version of yarn is 3.2.3. It may work on yarn 1 but I haven't confirmed.

Copy link
Member

Tobbe commented Sep 12, 2022

Took like five tries to get the windows tests to pass

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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants