-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: remove possible trailing slash from appName prompt #1466
Conversation
🦋 Changeset detectedLatest commit: f2a49bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@brunoeduardodev is attempting to deploy a commit to the t3-oss Team on Vercel. A member of the Team first needs to authorize it. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://create-t3-app-git-fork-brunoeduardodev-next-t3-oss.vercel.app/ |
I don't fully love this solution since there's a duplicated checking, both on the validate function and after getting the prompt result, do y'all have any suggestions on this? |
Currently the PR doesn't affect |
not sure about implementation details. but in terms of how it works for the user, it should be consistent for:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks really clean :)
one remaining issue is that it doesn't catch a name that is just /
and thus turned into an empty string, causing the command line to crash (if the user submits an empty string the normal way, their app gets named my_t3_app
instead).
this behaviour should definitely be taken care of, but not sure what's better:
- doing the same thing as when an empty string is provided at the start
- turning an app name of
/
into.
as it feels like this might be the user intent (they almost certainly didnt intend to init at the root of their file system) - an error of some sort
I see, currently the create-t3-app gives a validation error when a user prompts So my suggestion would be keeping this behavior, since there might be some kind of weird scenario where it's wanted to create the app through the root, this would mean we are going to remove trailing slashes only if there's at least 2 characters, do you have thoughts? |
Sounds good! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good :) Thanks for the PR!
* feat: remove possible trailing slash from appName prompt * chore: add changeset * feat: aslo remove trailing slash from args and ci * fix: only remove trailing slash when there's at least 2 characters * feat: removeTralingSlash file
Closes #1460
✅ Checklist
Changelog
Ignore trailing slashes when prompting the app name
Screenshots
When prompting
./
When prompting any path with a trailing slash
💯