-
-
Notifications
You must be signed in to change notification settings - Fork 336
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 flags for init
command
#1186
Conversation
🦋 Changeset detectedLatest commit: 0f15556 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 |
This will require a bump in the CLI version not should it go to v0.10.8? |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Thanks for taking the initiative to implement this! @AdrianGonz97 built/maintains the CLI so we'll wait for his review/comments before continuing forward! |
Co-authored-by: Hunter Johnston <64506580+huntabyte@users.noreply.github.com>
Any update on this? |
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 is excellent, thank you!
I've made a few changes regarding single dashed flags (uncommon options that aren't booleans shouldn't need them) as well as some stylistic tweaks. Overall though, this is great!
One thing that should be changed before merging is the validation order. I'd prefer if we frontloaded the validation for specified options, rather than holding it off until after the prompt is executed. For instance, if you were to do shadcn-svelte init --css invalid-path
, you wouldn't get the error until after the Which base color would you like to use?
prompt:
┌ shadcn-svelte v0.10.7
│
◇ Would you like to use TypeScript? (recommended)
│ Yes
│
◇ Which style would you like to use?
│ Default
│
◇ Which base color would you like to use?
│ Slate
│
└ [CLI Error]: "some-invalid-path" does not exist. Please enter a valid path.
It would be better to have that error popup at the start.
I've ran out of time to implement this as part of this PR. Would you be willing to update it? If not, no problem! I can get back to it later
Yeah I can add that no problem. I have a few minutes now but if not in the next few minutes I can work on it this evening. |
Hey so the validation for import aliases is a dependency of the TypeScript prompt? Is it okay if it goes after? Edit: See below const validateImportAlias = (alias: string) => {
// `tsconfigName` is from the `Would you like to use TypeScript?` prompt.
const tsconfig = cliConfig.getTSConfig(cwd, tsconfigName);
// -- snip --
}; |
Maybe it would be better to just automatically detect tsconfig / jsconfig? I can't see a reason why we couldn't just do that? |
We can detect with something like this: export function detectLanguage(cwd: string): "tsconfig.json" | "jsconfig.json" | undefined {
if (fs.existsSync(path.join(cwd, "tsconfig.json"))) {
return "tsconfig.json";
} else if (fs.existsSync(path.join(cwd, "jsconfig.json"))) {
return "jsconfig.json";
} else {
return undefined;
}
} Ideally this would make the CLI more ergonomic for normal users as well. |
Sure, feel free to extract that out into it's own utility function and have the
This would not be ideal as the |
Would it be possible to detect the config using getTSConfig? It seems like it is locating it based on the directory containing the Something like: export function detectLanguage(cwd: string): "tsconfig.json" | "jsconfig.json" | undefined {
if (getTsconfig(path.resolve(cwd, "package.json"), "tsconfig.json") != null) {
return "tsconfig.json";
} else if (getTsconfig(path.resolve(cwd, "package.json"), "jsconfig.json") != null) {
return "jsconfig.json";
} else {
return undefined;
}
} Edit: Fixed the function so it doesnt throw. I think theres a better way to do this to return the config with it but we could auto detect with a similar method no? |
Sorry for the spam I think this could work. This is exactly what the export function detectLanguage(cwd: string): "tsconfig.json" | "jsconfig.json" | undefined {
const rootPath = path.resolve(cwd, "package.json");
const tsConfig = getTSConfig(rootPath, "tsconfig.json");
if (tsConfig != null)
return tsConfig;
const jsConfig = getTSConfig(rootPath, "jsconfig.json");
if (jsConfig != null)
return jsConfig;
return undefined;
} |
No problem, I was just in the middle of writing a solution similar to that 🙂 I think that could work (will have to tweak the return type to be the config type, but same idea), go for it! |
Oops! Yeah I forgot to change that in my example. |
- Now auto-detects typescript/jsdoc config - Moves error messages to the start when provided by flags
We now auto detect TypeScript and that allows us to move the errors to the top when using flags. Updated changeset and docs accordingly. |
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.
Perfect, this is fantastic! Thanks so much for your work on this!!
No problem thanks for taking the time today! |
feat: Add flags for
init
commandinit
command #1183Adds new flags to allow for automated use of the CLI. This can be useful for anyone building templates around the library.
How it works
User provides the flags for each option. If an invalid flag is provided it will throw an error (this is to prevent hanging of runs in automation tools). If a flag is not provided the user will be prompted the same way as before including retrys.
New flags
New
--help
with the new flagsI had to make a few changes to make this work:
getStyles
andgetBaseColors
into sync functionsgetBaseColors
was already sync butgetStyles
needed to be changed to no longer fetch from the registryChecklist