handle exception when dependencies are not installed #191
Conversation
|
|
@illright could you please review? |
| console.error('\n\x1b[31mError: Unable to find Nuxt TypeScript configuration\x1b[0m') | ||
| console.error('\nThis appears to be a Nuxt project, but the TypeScript configuration files are missing.') | ||
| console.error('These files are auto-generated when you install dependencies.') | ||
| console.error('\nTo fix this:') | ||
| console.error('1. Run \x1b[36mnpm install\x1b[0m or \x1b[36myarn\x1b[0m or \x1b[36mpnpm install\x1b[0m') | ||
| console.error('2. Then run Steiger again\n') |
There was a problem hiding this comment.
suggestion: we have picocolors installed, let's use that instead of raw ANSI codes for text coloring for better readability
| devDependencies?: Record<string, string> | ||
| } | ||
|
|
||
| const readPackageJSON = (path: string) => JSON.parse(readFileSync(join(path, 'package.json'), 'utf-8')) as PackageJson |
There was a problem hiding this comment.
issue: it might be the case that package.json is in one of the parent folders, not necessarily the one where Steiger was run
suggestion: let's use empathic to locate the package.json file:
import * as find from 'empathic/find'
find.up('package.json', { cwd: path })
| } | ||
|
|
||
| export function handleError(error: unknown, context: ErrorContext): never { | ||
| const isHandled = [checkNuxtConfigError].some((checker) => checker(error, context)) |
There was a problem hiding this comment.
praise: interesting architecture! I'm not sure how many more of those there will be, but thanks for building with extensibility in mind :)
| } | ||
| } | ||
|
|
||
| export function handleError(error: unknown, context: ErrorContext): never { |
There was a problem hiding this comment.
issue: it takes a bit of code reading to understand what this function does, especially since its return type is never
suggestion: let's add a docstring that explains that this function attempts to gracefully handle the error using one of the known error checkers, and rethrows it if it can't
| const isHandled = [checkNuxtConfigError].some((checker) => checker(error, context)) | ||
|
|
||
| if (isHandled) { | ||
| process.exit(1) |
There was a problem hiding this comment.
issue: not a big fan of handling process exits from within the code of the linter app. This is more the responsibility of the CLI — to print the error message and exit with an error code
suggestion: maybe we can redo this function to instead throw a known error, and also export a separate function that would handle the error message printing? The way I see it working is like this:
- The linter code encounters a tsconfig error
- The
handleErrorfunction recognizes that this is a Nuxt-related issue - The linter code throws a
KnownIssueError(or something) to the caller, in this case,cli.ts - The CLI uses the the code from the
handle-errorfeature to print some guidance - The CLI returns with a unique error code
In this solution, I think it would also make sense to make handle-error a folder, and maybe also give it a more descriptive name like detect-known-exceptions
There was a problem hiding this comment.
suggestion: let's remove this and some other unnecessary template files: README.md, robots.txt
resolves #91