Bug: `exactOptionalPropertyTypes` causes type errors when using plugins that were not built with this option

[sebastian-altamirano]

Environment

Node version: v22.15.0 npm version: v11.3.0 Local ESLint version: v9.27.0 (Currently used) Global ESLint version: Not found Operating System: linux 5.15.167.4-microsoft-standard-WSL2

What parser are you using?

Default (Espree)

What did you do?

I have installed some plugins and configured ESLint to use them. ESLint works fine, but TypeScript throws some type errors related to exactOptionalPropertyTypes, which I have set to true in my tsconfig.json since I'm extending from @tsconfig/strictest.

What did you expect to happen?

There should be no type errors when using exactOptionalPropertyTypes.

What actually happened?

Type errors occurred.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/vitejs-vite-gantpukn?file=eslint.config.mjs,tsconfig.json&view=editor

Participation

• [x] I am willing to submit a pull request for this issue.

Additional comments

I have 2 reproductions:

• https://stackblitz.com/edit/vitejs-vite-gantpukn?file=eslint.config.mjs,tsconfig.json&view=editor: Contains a minimal reproducible example. It reproduces the error using a single plugin @eslint/markdown.
• https://stackblitz.com/edit/stackblitz-starters-rh2kntpu?file=eslint.config.mjs,tsconfig.config.json&view=editor: This is the configuration where I experienced the issue. It makes use of many plugins, so there are more type errors. You may notice that eslint does not work in this repro. This is because npm fails to install eslint-import-resolver-typescript, which requires native modules. It works if you download the code and run it locally. You can also make it work by force installing @unrs/resolver-binding-wasm32-wasi as suggested in https://github.com/import-js/eslint-import-resolver-typescript/issues/431. I did not do this because it breaks npm i and I did not want to break the repo.

According to @nzakas: "The Plugin type exported from eslint is the thing that needs updating to address this." (https://github.com/eslint/markdown/pull/391#issuecomment-2912790683)

[sebastian-altamirano]

If exactOptionalPropertyTypes is set to false at https://stackblitz.com/edit/stackblitz-starters-rh2kntpu?file=eslint.config.mjs,tsconfig.config.json&view=editor, there are still type errors.

These are for a different issue, but I’m unsure whether they fall under ESLint’s responsibility, the plugins' responsibility, or if my configuration needs adjustments. Any thoughts on where this should be addressed so that I can create the corresponding issues?

[fasttime]

Thanks for the issue @sebastian-altamirano. I can see that the @eslint/markdown types for the processor config include some optional properties with value undefined, which are inconsistent with how configs in ESLit are typed.

from https://app.unpkg.com/@eslint/[email protected]/files/dist/esm/index.d.ts#L184-213:

        processor: ({
            name: string;
            plugins: {};
            files?: undefined;
            processor?: undefined;
            languageOptions?: undefined;
            rules?: undefined;
        } | {
            name: string;
            files: string[];
            processor: string;
            plugins?: undefined;
            languageOptions?: undefined;
            rules?: undefined;
        } | {
            name: string;
            files: string[];
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        impliedStrict: boolean;
                    };
                };
            };
            rules: {
                [rule: string]: import("eslint").Linter.RuleEntry<any[]>;
            };
            plugins?: undefined;
            processor?: undefined;
        })[];

Note that files, processor, languageOptions, rules, plugins are explicitly typed with the value undefined in some of the config objects. To me, this looks like a tsc compiler artifact given that none of these properties are specified as undefined in the source code (https://github.com/eslint/markdown/blob/v6.4.0/src/index.js#L108-L134).

			{
				name: "markdown/recommended/plugin",
				plugins: (processorPlugins = {}),
			},
			{
				name: "markdown/recommended/processor",
				files: ["**/*.md"],
				processor: "markdown/markdown",
			},
			{
				name: "markdown/recommended/code-blocks",
				files: ["**/*.md/**"],
				languageOptions: {
					parserOptions: {
						ecmaFeatures: {
							// Adding a "use strict" directive at the top of
							// every code block is tedious and distracting, so
							// opt into strict mode parsing without the
							// directive.
							impliedStrict: true,
						},
					},
				},
				rules: {
					...processorRulesConfig,
				},
			},

[fasttime]

If exactOptionalPropertyTypes is set to false at https://stackblitz.com/edit/stackblitz-starters-rh2kntpu?file=eslint.config.mjs,tsconfig.config.json&view=editor, there are still type errors.

I tried running npm run check-eslint-config in the above repro, and I'm getting the same error messages with or without the exactOptionalPropertyTypes option set, so yes, these don't seem related to the behavior of that option in particular.

These are for a different issue, but I’m unsure whether they fall under ESLint’s responsibility, the plugins' responsibility, or if my configuration needs adjustments. Any thoughts on where this should be addressed so that I can create the corresponding issues?

If you encounter any incompatibilities with third party plugins it's better to file a bug at the plugin's maintainers, because ESLint doesn't provide the types for those plugins, and we can't support tools that we don't maintain.

Now, in the case of the eslint.config.mjs file in the StackBlitz repro, the compiler errors arise from the fact that some of the plugins in that config (angular-eslint, eslint-plugin-import-x, @smarttools/eslint-plugin-rxjs, typescript-eslint and possibly more) use type definitions provided by TSESlint, which are not compatible with native ESLint types. We had reports of similar incompatibilities in the past (e.g. eslint/eslint#19467). The inconsistencies are partly due to historical reasons, because typescript-eslint provided types for the flat config API before ESLint did.

[sebastian-altamirano]

Makes sense! Since @eslint/markdown does not add a type anotation for the plugin constant, TypeScript does its best to infer one, resulting in that unusual type with undefined values. Given this, I believe my original PR https://github.com/eslint/markdown/pull/391 makes more sense now.

Now, in the case of the eslint.config.mjs file in the StackBlitz repro, the compiler errors arise from the fact that some of the plugins in that config (angular-eslint, eslint-plugin-import-x, @smarttools/eslint-plugin-rxjs, typescript-eslint and possibly more) use type definitions provided by TSESlint, which are not compatible with native ESLint types. We had reports of similar incompatibilities in the past (e.g. https://github.com/eslint/eslint/issues/19467). The inconsistencies are partly due to historical reasons, because typescript-eslint provided types for the flat config API before ESLint did.

I did not know that! So that's why there are no errors when using tseslint.config().

[fasttime]

Sorry for the delay! I think there isn't much we can do address the incompatibilities with third-party plugins because of the above reasons. As for the issue with the eslint/markdown types, it should be probably addressed in that repo. The problem is that we don't use the exactOptionalPropertyTypes option to build the type declarations, and tsc creates types that are perfectly valid without that option. There are a few ways to fix this issue. Without adding excessive complexity to the build process, I can think of at least three solutions:

Type config explicitly

This approach would consist in coercing the type of the recommended-legacy config to LegacyConfig, and the type of the processor and recommended configs to Config[]. This can be achieved with a @type tag annotation as you did in eslint/markdown#391 e.g.:

    		processor: /** @type {Config[]} */ ([
			{
				name: "markdown/recommended/plugin",
				plugins: (processorPlugins = {}),
			},
			...
		),

The resulting type will be:

        processor: Config[];

This seems safe as the elements of individual config objects are not typically accessed directly.

Use a helper function

Another option is using a helper function to type the processor config as a tuple rather than as an array, as suggested in https://github.com/microsoft/TypeScript/issues/27179#issuecomment-422472039:

/**
 * @param {T} value
 * @template {unknown[]} T
 * @returns {T}
 */
function tuple(...value) {
	return value;
}

...

		processor: tuple(
			{
				name: "markdown/recommended/plugin",
				plugins: (processorPlugins = {}),
			},
			...
		),

The generated types will look like:

        processor: [{
            name: string;
            plugins: {};
        },
        ...
        ]

This is more correct than manual typing with Config[], but adds a function call.

Build type declarations with exactOptionalPropertyTypes

The exactOptionalPropertyTypes option requires strictNullChecks, which we don't use, because it would generate errors. The errors can be disabled with noCheck, but that would also turn off errors that we don't want to miss upon. This means that in order to fail the build with an error if the types aren't correct, we'll have to run tsc another time without noCheck but with noEmit, so that no types are generated. The whole command line would be like this:

npm run build:rules && rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json --noEmit && tsc -p tsconfig.esm.json --noCheck --exactOptionalPropertyTypes --strictNullChecks && npm run build:update-rules-docs

And the generated type for the processor config:

        processor: ({
            name: string;
            plugins: {};
            files?: never;
            processor?: never;
            languageOptions?: never;
            rules?: never;
        } |
        ...
        )

Note that undefined is now replaced by never. This is the most robust solution as it would avoid future incompatibilities that could arise because of exactOptionalPropertyTypes in that plugin.

[fasttime]

Moving to eslint/markdown. I would appreciate hearing the team's thoughts on how we should approach this. @eslint/eslint-team.

[nzakas]

@fasttime can you summarize? I can't quite follow the thread here.

[sebastian-altamirano]

The helper function is not needed:

const processorPluginConfig = {
	name: "markdown/recommended/plugin",
	plugins: (processorPlugins = {}),
};
const processorProcessorConfig = {
	name: "markdown/recommended/processor",
	files: ["**/*.md"],
	processor: "markdown/markdown",
};
const processorCodeBlocksConfig = {
	name: "markdown/recommended/code-blocks",
	files: ["**/*.md/**"],
	languageOptions: {
		parserOptions: {
			ecmaFeatures: {
				// Adding a "use strict" directive at the top of
				// every code block is tedious and distracting, so
				// opt into strict mode parsing without the
				// directive.
				impliedStrict: true,
			},
		},
	},
	rules: {
		...processorRulesConfig,
	},
};

const plugin = {
	configs: {
		// ...

		processor:
		 /**
		  * @type {[
		  *     typeof processorPluginConfig,
		  *     typeof processorProcessorConfig,
		  *     typeof processorCodeBlocksConfig
		  * ]}
		  */ ([
			processorPluginConfig,
			processorProcessorConfig,
			processorCodeBlocksConfig,
		]),
	}
}

But as you say, it does not avoid future incompatibilities.

[sebastian-altamirano]

What if exactOptionalPropertyTypes is added only to tests/types/tsconfig.json? This should help detect future incompatibilities. The tests already use a stricter configuration than the code.

[fasttime]

@fasttime can you summarize? I can't quite follow the thread here.

Understandable, since the original issue includes multiple problems. The specific problem concerning this repo is that the generated types aren't usable with ESLint when a user has the exactOptionalPropertyTypes option enabled in its TypeScript config. When this option is set, TypeScript does not allow assigning undefined to an optional property whose type does not include undefined. This is problematic for eslint/markdown because the autogenerated types for the processor config (plugin.configs.processor) contain some properties that are being inconsistently typed as undefined, for example files:

            files?: undefined;

(from https://app.unpkg.com/@eslint/[email protected]/files/dist/esm/index.d.ts#L187)

ESLint will throw an error if a config object has files explicitly set to undefined, and the types for Linter.Config["files"] reflect this fact by not including undefined explicitly:

		files?: Array<string | string[]>;

(from https://github.com/eslint/eslint/blob/v9.28.0/lib/types/index.d.ts#L1778)

The result is that plugin.configs.processor is not assignable to Linter.Config[] when exactOptionalPropertyTypes is on and ultimately, the plugin object is not assignable to ESLint.Plugin.

This isn't difficult to fix, but there are a few ways to do so, so I'd like to know what others think is the best solution.

[nzakas]

Thanks, that's very helpful. So my understanding is that tsc is generating a type for processor config that doesn't line up with the Config definition in the eslint package?

If that's so, couldn't we just type the export of index.js as ESLint.Plugin, like this? https://github.com/eslint/markdown/pull/408

[fasttime]

Thanks, that's very helpful. So my understanding is that tsc is generating a type for processor config that doesn't line up with the Config definition in the eslint package?

Correct. When exactOptionalPropertyTypes is set, the type for the processor config isn't in line with the Linter.Config definition in ESLint.

If that's so, couldn't we just type the export of index.js as ESLint.Plugin, like this? #408

ESLint.Plugin is a base type for all plugins. It does not include type information for accessing specific objects defined in @eslint/markdown (or any other plugin), like configs, rules, and so forth. Linter.Config is also a generic base type for config objects, but since the elements config objects aren't typically accessed individually, it should be safe to type each config array with Linter.Config[], if that is the approach we prefer to take.

[sebastian-altamirano]

...it should be safe to type each config array with Linter.Config[], if that is the approach we prefer to take.

I think using a function, as you suggested, or using constants and typeof, as I suggested, is preferable since the configuration will be accessed when extending using map, for example:

import markdown from '@eslint/markdown';
import { defineConfig } from 'eslint/config';

export default defineConfig([
  markdown.configs.recommended.map((config) => ({
    ...config,
    language: 'markdown/gfm',
    rules: {
      ...config.rules,
      'markdown/no-duplicate-headings': 'error',
      'markdown/no-html': 'error',
    },
  })),
]);

This example is not the best because the recommended configuration has a flat structure (also, the processor configuration is the one causing the issue), but the processor configuration has nested objects, so the DX will worsen if someone wants to access them.

I know that defineConfig has extends now, but for backwards compatibility and consistency, I think this is a better solution. I have inspected other official plugins, and they all seem to agree to let TS infer the types instead of using the ones provided by ESLint.

[nzakas]

I'd much prefer a solution that doesn't put the responsibility for this on plugin authors, and if it must be on them, then to be as light-touch as possible.

Is there a reason we can't redefined the files property in ESLint to allow undefined? Wouldn't that solve it without needing plugins to make changes?

[fasttime]

I'd much prefer a solution that doesn't put the responsibility for this on plugin authors, and if it must be on them, then to be as light-touch as possible.

For clarity, only @eslint/markdown is affected by the problem with exactOptionalPropertyTypes. The other plugins in the repro linked in the description are failing the type check for unrelated reasons.

Is there a reason we can't redefined the files property in ESLint to allow undefined? Wouldn't that solve it without needing plugins to make changes?

If we allow undefined in Linter.Config["files"], TS users will be no longer notified by the compiler if they accidentally set files to a potentially undefined value in their configs, which could happen because of a mistake in the logic. This somewhat undermines the purpose of exactOptionalPropertyTypes, which aims to catch these types of errors early on. If you prefer this approach, I recommend modifying all optional properties in the ESLint types to consistently include undefined. This will prevent other similar issues with new projects that have exactOptionalPropertyTypes activated.

[nzakas]

It seems like the properties of LegacyConfig already follow this pattern of optional properties also being set to undefined: https://github.com/eslint/eslint/blob/main/lib/types/index.d.ts#L1453

TS users will be no longer notified by the compiler if they accidentally set files to a potentially undefined value in their configs, which could happen because of a mistake in the logic.

How important is this?

@mdjermanovic what do you think?

[mdjermanovic]

TS users will be no longer notified by the compiler if they accidentally set files to a potentially undefined value in their configs, which could happen because of a mistake in the logic.

How important is this?

@mdjermanovic what do you think?

If we don't intend to allow undefined values in runtime, then I think it is important that the types also don't allow them, so that users are notified when attempting to set a value that is potentially undefined, as that's what types are for.

[nzakas]

This is where I'm not quite sure what is idiomatic for TypeScript. If a property is optional, then setting it to undefined seems like the same thing to me. As already noted, this is how LegacyConfig already works.

In any case, I'd like a solution that doesn't require every plugin to do strange things just to get its types to validate. I don't find any of the solutions listed in https://github.com/eslint/markdown/issues/402#issuecomment-2930878167 satisfying.

Is this something we can solve with definePlugin?

cc @JoshuaKGoldberg

[lishaduck]

~~This is where I'm not quite sure what is idiomatic for TypeScript.~~

It's been recommended for a while, it'll be on by default for new projects starting in TS 5.9: https://github.com/microsoft/TypeScript/issues/58420.

EDIT: I think I misread that comment, but I still think that it's worth noting. Also, here's how I got html-eslint to infer correctly: https://github.com/yeonjuan/html-eslint/blob/main/packages/eslint-plugin/lib/index.js. Doesn't seem too weird to me, beyond the getter?

[JoshuaKGoldberg]

If a property is optional, then setting it to undefined seems like the same thing to me.

The difference being picked up on by exactOptionalPropertyTypes is whether:

• The object does not have the key at all
• The object has the key, and the corresponding value is undefined

That difference doesn't matter often in real-world code. Hence it not being on by default in TypeScript. But making a distinction is more technically correct.

+1 to what lishaduck noted: even though a lot of existing projects don't use exactOptionalPropertyTypes, it's going to get more and more popular over time. And a nontrivial % of projects use it already. My general opinion is that it'd be best to change the exported types in esm/index.d.ts to be more precise, so as to fix type errors for users who enable exactOptionalPropertyTypes.

Build type declarations with exactOptionalPropertyTypes

I think this would be the optimal strategy. I'm not sure I follow the full comment though @fasttime, is there any reason not to want to turn exactOptionalPropertyTypes on always?

Note that practically, it's blocked on also enabling strictNullChecks. Which causes 79 type errors when enabled. Most of them are from @types/unist indicating node properties as undefined. Which is a separate issue: the types should mark a difference between "standard node with position" vs. "generated node that doesn't have a position".

Related conversation about enabling some of the strict flags in ESLint code: https://github.com/eslint/rewrite/issues/118

Failing enabling new compiler options...

Type config explicitly

This seems like a reasonable good next step to me. Confirmed it fixes & simplifies the generated esm/index.d.ts for me locally.

[fasttime]

is there any reason not to want to turn exactOptionalPropertyTypes on always?

Note that practically, it's blocked on also enabling strictNullChecks. Which causes 79 type errors when enabled. Most of them are from @types/unist indicating node properties as undefined.

This is pretty much the main reason why exactOptionalPropertyTypes can't be enabled by default, I think. Enabling strictNullChecks would require changes in multiple repos. That would fix the bug described in this issue, but it seems to me that there are simplex methods to accomplish this particular fix.

[lumirlumir]

I believe the same issue exists in the CSS and JSON plugins as well.

When strictNullChecks is enabled, the same problem occurs in those plugins too.

[nzakas]

@JoshuaKGoldberg do you think that the definePlugin() function could help with this problem?

[JoshuaKGoldberg]

I do think so, yes. Or at least it should. I'm glad we're having this discussion now; IMO this is a good note -> test case to add to https://github.com/eslint/rfcs/pull/132.

[nzakas]

Then I'd prefer to hold off until we finalize definePlugin() to see if that can address this issue holistically.

As mentioned previously, I really don't like forcing every plugin developer to jump through hoops to get the types correct. This seems like something that should "just work" out of the box in some way.

[JoshuaKGoldberg]

I added a Plugin Testing section to https://github.com/eslint/rfcs/pull/132 noting this issue. https://github.com/eslint/rfcs/pull/132/commits/6f385015ff0664ff4c607ba66c69aecad813d8bb

[lishaduck]

I kept thinking about this because I remembered there was an issue with setting a plugin to be of type ESLint.Plugin, and just remembered: you lose the type of configs, which is a bummer, so you'd probably need to make ESLint.Plugin generic over configs, and then make definePlugin handle that (should be non-breaking).

EDIT: And remembered the issue with that: which then means that the types of configs is part of the types, which is good for distinguishing between arrays and objects, but otherwise should be impl details (I've hit issues with config types being too specific and conflicting with what will be https://github.com/eslint/eslint/issues/19721).

[github-actions[bot]]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[lishaduck]

Not stale

[nzakas]

Not stale, still looking at this.