-
Notifications
You must be signed in to change notification settings - Fork 6
Improve detection and reporting of missing TS launcher (ts-node) #56
Description
For script launchers that aren't node itself (currently only ts-node for TS commands), we use npm ls to look up whether the launcher package is installed/available, and notify the user about its absence otherwise.
However, #55 revealed that this has two problems:
npm lsreturns non-zero not only when the package is missing, but also when it is "extraneous" (present in node_modules but not in package.json).- Apparently it is also noticably slow in certain situations (probably depending on size of node_modules and disk performance)
For problem 1., we should consider whether we want to keep this behavior -- having extraneous packages is an inconsistent state, hence npm ls showing it as an error, but technically we could still try to execute our command.
If we keep it, we should improve the output because we're currently always reporting the "missing" case even if the package is extraneously present. If we want to allow extraneously present ts-node for DevCmd, we should use a different method of detection.
Taking problem 2. into account, we should also consider whether we always want to check for ts-node presence pre-emptively, or if it's enough to check it when spawning it fails.
For reference, the code using npm ls is:
devcmd/packages/devcmd/src/utils/npm_utils.ts
Lines 4 to 12 in 8d4902b
| export async function checkPackageAvailable(packageName: string, directory: string): Promise<boolean> { | |
| return new Promise<boolean>((res) => { | |
| // `npm ls <package_name>` only exits with status code 0 when <package_name> is available | |
| const childProcess = spawn(withCmdOnWin("npm"), ["ls", packageName], { cwd: directory }); | |
| childProcess.on("error", (): void => res(false)); | |
| childProcess.on("close", (code: number): void => res(code === 0)); | |
| }); | |
| } |
And it is invoked from here (line 144):
devcmd/packages/devcmd/src/from-cli/index.ts
Lines 136 to 159 in 8d4902b
| async function tryRunScript( | |
| devCmdsDir: string, | |
| scriptFilepath: string, | |
| commandArgs: string[], | |
| extension: string, | |
| launcher: string | |
| ): Promise<boolean> { | |
| if (await isFile(scriptFilepath)) { | |
| if (extension === "ts" && !(await checkPackageAvailable(launcher, devCmdsDir))) { | |
| throw new Error(`No script runner for TypeScript devcmds found. Did you forget to install ${bold(launcher)}?`); | |
| } | |
| // TODO: use spawn or so instead | |
| const { status } = spawnSync(withCmdOnWin(launcher), [scriptFilepath, ...commandArgs], { | |
| stdio: "inherit", | |
| cwd: devCmdsDir, | |
| }); | |
| if (status !== null && status != 0) throw new Error(`Process failed with exit code ${status}`); | |
| return true; | |
| } | |
| return false; | |
| } |