tsdjs / tsd

Check TypeScript type definitions
MIT License
2.36k stars 68 forks source link

fix: don't log stacktrace if cli succeeds (regression) #182

Closed tommy-mitchell closed 1 year ago

tommy-mitchell commented 1 year ago

Starting in v0.26.1, successful runs of the CLI (i.e. when it doesn't crash) incorrectly report an error and error stack while running tsd:

Error running tsd: Error: 
  index.test-d.ts:26:0
  ✖  25:0  Expected an error, but found none.  
  ✖  26:0  ) expected.                         

  2 errors

    at ~/node_modules/tsd/dist/cli.js:64:19
    at Generator.next (<anonymous>)
    at fulfilled (~/node_modules/tsd/dist/cli.js:6:58)

This PR restores the old behavior and makes sure that the error message and stack are only displayed when tsd crashes.

tommy-mitchell commented 1 year ago

I’m not sure why the tests are failing in Node 14.

tommy-mitchell commented 1 year ago

Turns out error.cause isn't supported in Node 14:

throw new Error(formatter(diagnostics, showDiff), {cause: 'tsd found diagnostics'});
tommy-mitchell commented 1 year ago

It now only throws on failure, and otherwise exits with an error code if any error-level diagnostics are found:

const exit = (message: string, {isError = true}: {isError?: boolean} = {}) => {
    if (isError) {
        console.error(message);
        process.exit(1);
    } else {
        console.log(message);
        process.exit(0);
    }
};

try {
    // ...
    if (diagnostics.length > 0) {
        const hasErrors = diagnostics.some(diagnostic => diagnostic.severity === 'error');
        const formattedDiagnostics = formatter(diagnostics, showDiff);

        exit(formattedDiagnostics, {isError: hasErrors});
    }
} catch (error: unknown) {
    const potentialError = error as Error | undefined;
    const errorMessage = potentialError?.stack ?? potentialError?.message ?? 'tsd unexpectedly crashed.';

    exit(`Error running tsd:\n${errorMessage}`);
}
tommy-mitchell commented 1 year ago

The formatter also needs to report the number of warnings, but I have another PR for that.

Edit: #184