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

feat: improved errors when config can not be found #85

Merged
merged 2 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import process from 'node:process'
import fs from 'node:fs/promises'
import { existsSync } from 'node:fs'
import { relative, resolve } from 'node:path'
import { types } from 'node:util'

import open from 'open'
import { getPort } from 'get-port-please'
import cac from 'cac'
Expand Down Expand Up @@ -40,6 +42,11 @@ cli
globMatchedFiles: options.files,
})

if (types.isNativeError(configs)) {
configs.prettyPrint()
process.exit(1)
}

let baseURL = options.base
if (!baseURL.endsWith('/'))
baseURL += '/'
Expand Down
81 changes: 72 additions & 9 deletions src/configs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { dirname, relative, resolve } from 'node:path'
import { basename, dirname, relative, resolve } from 'node:path'
import process from 'node:process'
import { types } from 'node:util'
import { ConfigArray } from '@eslint/config-array'
import { configArrayFindFiles } from '@voxpelli/config-array-find-files'
import { bundleRequire } from 'bundle-require'
Expand All @@ -8,7 +9,7 @@ import c from 'picocolors'
import { resolve as resolveModule } from 'mlly'
import type { FlatConfigItem, MatchedFile, Payload, RuleInfo } from '../shared/types'
import { isIgnoreOnlyConfig, matchFile } from '../shared/configs'
import { MARK_CHECK, MARK_INFO, configFilenames } from './constants'
import { MARK_CHECK, MARK_ERROR, MARK_INFO, configFilenames, legacyConfigFilenames } from './constants'

export interface ReadConfigOptions extends ResolveConfigPathOptions {
/**
Expand Down Expand Up @@ -40,6 +41,47 @@ export interface ResolveConfigPathOptions {
userBasePath?: string
}

export class ConfigPathError extends Error {
override name = 'ConfigPathError' as const

constructor(
public basePath: string,
public configFilenames: string[],
) {
super('Cannot find ESLint config file')
}

prettyPrint() {
console.error(MARK_ERROR, this.message, c.dim(`

Looked in ${c.underline(this.basePath)} and parent folders for:

* ${this.configFilenames.join('\n * ')}`,
))
}
}

export class ConfigPathLegacyError extends Error {
override name = 'ConfigPathLegacyError' as const

constructor(
public basePath: string,
public configFilename: string,
) {
super('Found ESLint legacy config file')
}

prettyPrint() {
console.error(MARK_ERROR, this.message, c.dim(`

Encountered unsupported legacy config ${c.underline(this.configFilename)} in ${c.underline(this.basePath)}

\`@eslint/config-inspector\` only works with the new flat config format:
https://eslint.org/docs/latest/use/configure/configuration-files-new`,
))
}
}

/**
* Search and read the ESLint config file.
*
Expand All @@ -55,12 +97,27 @@ export async function resolveConfigPath(options: ResolveConfigPathOptions) {
if (userBasePath)
userBasePath = resolve(cwd, userBasePath)

const configPath = userConfigPath
? resolve(cwd, userConfigPath)
: await findUp(configFilenames, { cwd: userBasePath || cwd })
const lookupBasePath = userBasePath || cwd

let configPath = userConfigPath && resolve(cwd, userConfigPath)

if (!configPath) {
configPath = await findUp(configFilenames, { cwd: lookupBasePath })
}

if (!configPath) {
const legacyConfigPath = await findUp(legacyConfigFilenames, { cwd: lookupBasePath })

if (!configPath)
throw new Error('Cannot find ESLint config file')
return legacyConfigPath
? new ConfigPathLegacyError(
`${relative(cwd, dirname(legacyConfigPath))}/`,
basename(legacyConfigPath),
)
: new ConfigPathError(
`${relative(cwd, lookupBasePath)}/`,
configFilenames,
)
}

const basePath = userBasePath || (
userConfigPath
Expand All @@ -83,13 +140,19 @@ export async function resolveConfigPath(options: ResolveConfigPathOptions) {
*/
export async function readConfig(
options: ReadConfigOptions,
): Promise<{ configs: FlatConfigItem[], payload: Payload, dependencies: string[] }> {
): Promise<ConfigPathLegacyError | ConfigPathError | { configs: FlatConfigItem[], payload: Payload, dependencies: string[] }> {
const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we use throw instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antfu As throws are not typed in TypeScript and can be intertwined with other kinds of exceptions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could throw and use if (error instanceof ConfigPathLegacyError) to do the type guard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but not sure it would make the code simpler :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it would make it more complicated either - but I think the point is that errors are supposed to be thrown, and loadConfig by it's name should returns the config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that I never got to fixing this 🙈

Speaking on this topic, ran into this standard proposal the other day: https://github.com/arthurfiorette/proposal-safe-assignment-operator

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries at all.

Yeah I saw it. Would love to see it landed someday!

chdir = true,
globMatchedFiles: globFiles = true,
} = options

const { basePath, configPath } = await resolveConfigPath(options)
const resolvedConfigPath = await resolveConfigPath(options)

if (types.isNativeError(resolvedConfigPath)) {
return resolvedConfigPath
}

const { basePath, configPath } = resolvedConfigPath
if (chdir && basePath !== process.cwd())
process.chdir(basePath)

Expand Down
9 changes: 9 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,14 @@ export const configFilenames = [
'eslint.config.cts',
]

export const legacyConfigFilenames = [
'.eslintrc.js',
'.eslintrc.cjs',
'.eslintrc.yaml',
'.eslintrc.yml',
'.eslintrc.json',
]

export const MARK_CHECK = c.green('✔')
export const MARK_INFO = c.blue('ℹ')
export const MARK_ERROR = c.red('✖')
16 changes: 15 additions & 1 deletion src/ws.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import process from 'node:process'
import { types } from 'node:util'

import chokidar from 'chokidar'
import type { WebSocket } from 'ws'
import { WebSocketServer } from 'ws'
Expand Down Expand Up @@ -27,7 +30,15 @@ export async function createWsServer(options: CreateWsServerOptions) {
ws.on('close', () => wsClients.delete(ws))
})

const { basePath } = await resolveConfigPath(options)
const resolvedConfigPath = await resolveConfigPath(options)

if (types.isNativeError(resolvedConfigPath)) {
resolvedConfigPath.prettyPrint()
process.exit(1)
}

const { basePath } = resolvedConfigPath

const watcher = chokidar.watch([], {
ignoreInitial: true,
cwd: basePath,
Expand All @@ -51,6 +62,9 @@ export async function createWsServer(options: CreateWsServerOptions) {
if (!payload) {
return await readConfig(options)
.then((res) => {
if (types.isNativeError(res)) {
throw res
}
const _payload = payload = res.payload
_payload.meta.wsPort = port
watcher.add(res.dependencies)
Expand Down
Loading