Add folder as argument to create-toolpad-app
#1795
Conversation
|
Some questions that popped in my head after reading this
|
I think that depends on whether the option is advertised as a path or a name; in the former case I expect that We can expand the scope of this PR to include |
👍 If by "option" you mean "first positional argument" and not |
Yes ✅ |
| // eslint-disable-next-line no-await-in-loop | ||
| projectName = await scaffoldProject(name.projectName, cwd); | ||
| // eslint-disable-next-line no-await-in-loop | ||
| projectName = await scaffoldProject(name.projectName, cwd); |
There was a problem hiding this comment.
Maybe assign the first argument you're passing to scaffoldProject to a variable so you don't need to write almost the same expression twice (so you can call scaffoldProject in just one line)?
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
| } while (!projectName); | ||
| absolutePath = await scaffoldProject(absolutePath); | ||
| count += 1; | ||
| } while (!absolutePath); |
There was a problem hiding this comment.
Is it possible to use the inquirer validate function to check the path before scaffolding the project? That way we can avoid the complex control flow above with the do/while and process.exit.
| const absolutePath = path.join(process.cwd(), relativePath); | ||
|
|
||
| try { | ||
| await fs.mkdir(absolutePath); |
There was a problem hiding this comment.
| await fs.mkdir(absolutePath); | |
| await fs.mkdir(absolutePath, { recursive: true }); |
There was a problem hiding this comment.
From https://nodejs.org/api/fs.html#fspromisesmkdirpath-options
Calling fsPromises.mkdir() when path is a directory that exists results in a rejection only when recursive is false
I want to be able to detect when a directory exists on calling mkdir so that I am able to catch that error and then call isDirectoryEmpty on the existing directory, but setting recursive to true causes me to miss that case
There was a problem hiding this comment.
Won't you need recursive if you pass a nested directory to the CLI? e.g.
yarn create toolpad-app some/nested/dir
I'd just validate the path before calling fs.mkdir instead of after
|
|
||
| return true; | ||
| } catch { | ||
| // Directory exists, verify if it's empty to proceed |
There was a problem hiding this comment.
Directory exists
Not necessarily, to be sure, check the error code and rethrow if it's not EEXIST
| absolutePath, | ||
| )} contains files that could conflict. Either use a new directory, or remove conflicting files.`; | ||
| } catch { | ||
| return `${chalk.red('error')} - Unable to access directory at ${chalk.red( |
There was a problem hiding this comment.
This way there is no feedback to the user about what went wrong exactly, since it's unexpected, I'd probably just let the error bubble up and crash the program.
| return `${chalk.red('error')} - The directory at ${chalk.blue( | ||
| absolutePath, | ||
| )} contains files that could conflict. Either use a new directory, or remove conflicting files.`; | ||
| } catch (error: any) { |
There was a problem hiding this comment.
Avoid any.
| } catch (error: any) { | |
| } catch (rawError: unknown) { | |
| const error = errorFrom(rawError); |
There was a problem hiding this comment.
Do you recommend importing errorFrom from @mui/toolpad-core or rewriting that function here?
There was a problem hiding this comment.
Added something local to avoid adding dependencies to this package
create-toolpad-appto have the project name be accepted as a runtime argument