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

Improve detection and reporting of missing TS launcher (ts-node) #56

Open
PatrickLehnerXI opened this issue Jan 31, 2022 · 0 comments
Open

Comments

@PatrickLehnerXI
Copy link
Contributor

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:

  1. npm ls returns non-zero not only when the package is missing, but also when it is "extraneous" (present in node_modules but not in package.json).
  2. 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:

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):

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;
}

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

No branches or pull requests

1 participant