Handling edge case bugs of create app command#537
Handling edge case bugs of create app command#537spenserhale wants to merge 3 commits intobitovi:mainfrom
Conversation
Updating README to have version check command print both node and npm versions in one go. Currently when you copy command and paste into terminal only prints one.
Updating Constants for Backends and Databases to provide the preferred option as the default
Fixing three bugs: 1) If your project name has spaces, the command will fail. Now uses packageName as folder name. 2) npm create vite@latest never runs on node version 20+. Now uses exec command instead of spawn.sync. 3) Reading the template package.json asks for file before the file is fully written to OS. Now has new function readFileWithRetries to wait for file is ready.
| throw new Error(`${filePath} could not be found after ${retries} attempts`); | ||
| } | ||
|
|
||
| export function execCommandExitOnError( |
There was a problem hiding this comment.
Should we remove the existing runCommand that is using spawn.sync and the cross-spawn import then?
There was a problem hiding this comment.
I did the exec since it spawns a new process, as there might be a bug with spawn.sync because it tries to reuse the same shell. It is the reason I believe 'npm create vite@latest' was failing, but is more of a hunch than 100% confirmed.
I haven't tested on Windows, so likely the better approach would be to see if cross-spawn is needed for Windows support and then decide if the project can just use the simpler standard library function or if the third-party library is needed, which in that case, the execCommandExitOnError should be rewritten to use cross-spawn for compatibility.
| cwd, | ||
| true, | ||
| ) | ||
| execCommandExitOnError(`npm create vite@latest "${targetDir}" -- --template react-ts`) |
There was a problem hiding this comment.
Would adding await here save the need to retry reading?
There was a problem hiding this comment.
You might be able to code with a then or a delay, but the core reason for retry is to avoid any I/O delays with the OS that could happen when the node asks for a file but the OS has not fully registered filesystem changes. For example, if you are working within a virtual or symlinked folder, there can be a delay between a written file and when you can read it, and we want to handle edge cases to make the program more robust.
2 participants
Fixing three bugs:
Other Changes:
Updating CLI defaults to order match preferred, and making CLI version check a one-liner.
Tested on Node 20, 18.