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

Is the readme correct regarding the required configuration? #39

Open
fregante opened this issue Jul 12, 2021 · 6 comments
Open

Is the readme correct regarding the required configuration? #39

fregante opened this issue Jul 12, 2021 · 6 comments

Comments

@fregante
Copy link
Member

{
	"name": "my-awesome-project",
	"eslintConfig": {
		"extends": [
			"xo",
			"xo-typescript"
		]
	}
}

I haven't tested fully, but it seems that this is not enough to enable the configuration on TS/TSX files. I had to call eslint src --ext ts to have the errors show up. This was not required when I was extending plugin:@typescript-eslint/* directly

@fregante
Copy link
Member Author

This enables XO’s config on TS files as well:

  "extends": [
    "plugin:@typescript-eslint/eslint-recommended",
    "xo",
    "xo-typescript"
  ],

I don't see anything special about it here: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/eslint-recommended.ts

@sindresorhus
Copy link
Member

That's weird. I don't see anything regarding extensions in the typescript-eslint config.

@fregante
Copy link
Member Author

fregante commented Jul 13, 2021

Weird indeed. Also confirmed with this repro:

npm init -y
npm install eslint eslint-config-xo eslint-config-xo-typescript @typescript-eslint/parser @typescript-eslint/eslint-plugin
echo '// @ts-ignore' > index.ts
echo '{}' > tsconfig.json
echo '{"extends":["xo","xo-typescript"]}' > .eslintrc
npx eslint .

Then this config produces:

Oops! Something went wrong! :(

ESLint: 7.30.0

No files matching the pattern "." were found.
Please check for typing mistakes in the pattern.

Add that the plugin and run again:

echo '{"extends":["plugin:@typescript-eslint/recommended","xo","xo-typescript"]}' > .eslintrc
npx eslint .
./index.ts
  1:1  error  Use "@ts-expect-error" to ensure an error is actually being suppressed  @typescript-eslint/prefer-ts-expect-error
  1:1  error  Do not use "@ts-ignore" because it alters compilation errors            @typescript-eslint/ban-ts-comment

✖ 2 problems (2 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Then remove XO’s config:

echo '{"extends":["plugin:@typescript-eslint/recommended"]}' > .eslintrc
npx eslint .
./index.ts
  1:1  error  Do not use "@ts-ignore" because it alters compilation errors  @typescript-eslint/ban-ts-comment

✖ 1 problem (1 error, 0 warnings)

@fregante
Copy link
Member Author

Worth an update of docs and config everywhere? typescript-eslint/typescript-eslint#3824 (comment)

@sindresorhus
Copy link
Member

Sure

@fregante
Copy link
Member Author

fregante commented Nov 11, 2023

I think I know why, and I think this config should follow suit:

https://github.com/typescript-eslint/typescript-eslint/blob/a8830c681f53af4e23a2cda231b3300159b18e7d/packages/eslint-plugin/src/configs/eslint-recommended.ts#L9

The rules are defined in an override for .ts files, specifying those extensions, whereas eslint-config-xo-typescript just loads all rules in the top level.

If the change isn't made here, I think the user should set up the config as:

{
	"name": "my-awesome-project",
	"eslintConfig": {
		"extends": [
			"xo",
		],
		overrides: [{
			files: ['*.ts', '*.tsx', '*.mts', '*.cts'],
			"extends": [
				"xo-typescript"
			]
		}]
	}
}

Note that this means users will also need to specify TypeScript-related rule changes in an override, if they have any. For example this won't work:

{
	"name": "my-awesome-project",
	"eslintConfig": {
		"extends": [
			"xo",
		],
+		"rules": {
+			"@typescript-eslint/no-unsafe-assignment": "warn" // Must be in overrides too
+		},
		overrides: [{
			files: ['*.ts', '*.tsx', '*.mts', '*.cts'],
			"extends": [
				"xo-typescript"
			],
		}]
	}
}

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

2 participants