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

[cli] Handle existing apps in instant-cli init #528

Merged
merged 1 commit into from
Nov 22, 2024
Merged

[cli] Handle existing apps in instant-cli init #528

merged 1 commit into from
Nov 22, 2024

Conversation

stopachka
Copy link
Contributor

@stopachka stopachka commented Nov 22, 2024

instant-cli init used to only handle the case where a user wanted to create a brand new app.

But most of the time, users already created an app in the dashboard. For this case, they would have to know that the right command would be to call instant-cli pull with an app id param.

Instead, I updated the instant-cli init flow, so we ask the user if they want to create a new app, or import an existing one.

Future work

  • In instant-cli pull, if no app id is provided, we should just prompt the user for an app id, like here
  • In instant-cli init, we should try to detect the app id. If we do, we should ask them if they want to import that app

Here's how things look:

First, ask: create a new app, or import an existing one?

image

If creating a new app, go through this flow:

image

Or, prompt for an app id

CleanShot 2024-11-22 at 11 30 01@2x

@dwwoelfel @nezaj

Copy link

View Vercel preview at instant-www-js-cli4-jsv.vercel.app.

roar

roar

roar
required: true,
}).catch(() => null);
const id = _id?.trim();
if (!id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth validating the input is a uuid? I wonder if someone may have a copy/paste error and would be nice if we can tell them can't find an app id?

(Maybe the error from pull hints this? I think right now the error message is terse?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I actually have a follow on PR which removes the manual prompt here entirely:

#532

I also check now when app id is imported from env, whether it's a UUID or not, so the user should get a good error message 🫡

Copy link
Contributor

@nezaj nezaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@stopachka stopachka merged commit 67ae4f7 into main Nov 22, 2024
26 checks passed
@stopachka stopachka deleted the cli4 branch November 22, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants