fix: type compatibility issue between Markdown v8 and ESLint v9.39.x#648
fix: type compatibility issue between Markdown v8 and ESLint v9.39.x#648lumirlumir wants to merge 19 commits into
Conversation
b8b4ffd to
0917752
Compare
0917752 to
38fd019
Compare
…solve-eslint-markdown-v8-and-eslint-v9-compatibility-issue
There was a problem hiding this comment.
Pull request overview
Fixes TypeScript type compatibility between @eslint/markdown v8 and ESLint v9.39.x by decoupling the published rule types from ESLint’s evolving internal rule-definition typings, while expanding CI/type checks across supported ESLint versions.
Changes:
- Switch JSDoc typing in runtime JS (
src/index.js,src/processor.js) to use types from@eslint/coreinstead ofeslint. - Update rule build generation to emit a
Record<RuleId, any>-typed rules map to avoid cross-major ESLint type incompatibilities. - Expand type-compatibility testing (CI matrix +
tests/types) and clarify README version guidance.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/build-rules.js | Generates a typed RuleId union and coerces exported rules map to a looser type to avoid ESLint type coupling. |
| tests/types/types.test.ts | Adds type assertions intended to validate compatibility with ESLint v9.39.x/v9.x/v10.x and rule key presence. |
| src/processor.js | Replaces eslint-sourced types with @eslint/core types in JSDoc for fixes/messages/ranges. |
| src/index.js | Replaces eslint-sourced config/rules typing with @eslint/core equivalents for generated declarations. |
| README.md | Updates installation note to clarify which ESLint versions are type-compatible. |
| .github/workflows/ci.yml | Adds an ESLint version matrix for the type-checking job and installs matching ESLint versions in CI. |
Comments suppressed due to low confidence (1)
src/processor.js:445
excludeUnsatisfiableRules()is used aftergroup.map(adjust)whereadjustcan returnnull, but the parameter is typed asLintMessage. Consider typing the parameter asLintMessage | nulland/or making this a type guard sopostprocess()is correctly typed as returningLintMessage[].
/**
* Excludes unsatisfiable rules from the list of messages.
* @param {LintMessage} message A message from the linter.
* @returns {boolean} True if the message should be included in output.
*/
function excludeUnsatisfiableRules(message) {
return message && !UNSATISFIABLE_RULES.has(message.ruleId);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…solve-eslint-markdown-v8-and-eslint-v9-compatibility-issue
|
Thanks for looking into this. I can confirm that this change resolves TypeScript compiler errors with ESLint v9.39.x. I'm wondering if it will be still possible for consumers to access the rule types after this PR. For example, currently you could get the type of the import plugin from "@eslint/markdown";
type NoHtmlRuleDefinition = typeof plugin.rules["no-html"];But this would stop working with this change. Is there another way to access the rule types that I'm missing? |
This comment was marked as outdated.
This comment was marked as outdated.
…solve-eslint-markdown-v8-and-eslint-v9-compatibility-issue
b350555 to
d65172f
Compare
fc0ec6a to
3cf80ce
Compare
First, I updated the following auto-generated built file from the first version to the second version. import rule0 from "../rules/fenced-code-language.js";
import rule1 from "../rules/fenced-code-meta.js";
import rule2 from "../rules/heading-increment.js";
import rule3 from "../rules/no-bare-urls.js";
import rule4 from "../rules/no-duplicate-definitions.js";
import rule5 from "../rules/no-duplicate-headings.js";
import rule6 from "../rules/no-empty-definitions.js";
import rule7 from "../rules/no-empty-images.js";
import rule8 from "../rules/no-empty-links.js";
import rule9 from "../rules/no-html.js";
import rule10 from "../rules/no-invalid-label-refs.js";
import rule11 from "../rules/no-missing-atx-heading-space.js";
import rule12 from "../rules/no-missing-label-refs.js";
import rule13 from "../rules/no-missing-link-fragments.js";
import rule14 from "../rules/no-multiple-h1.js";
import rule15 from "../rules/no-reference-like-urls.js";
import rule16 from "../rules/no-reversed-media-syntax.js";
import rule17 from "../rules/no-space-in-emphasis.js";
import rule18 from "../rules/no-unused-definitions.js";
import rule19 from "../rules/require-alt-text.js";
import rule20 from "../rules/table-column-count.js";
export default {
"fenced-code-language": rule0,
"fenced-code-meta": rule1,
"heading-increment": rule2,
"no-bare-urls": rule3,
"no-duplicate-definitions": rule4,
"no-duplicate-headings": rule5,
"no-empty-definitions": rule6,
"no-empty-images": rule7,
"no-empty-links": rule8,
"no-html": rule9,
"no-invalid-label-refs": rule10,
"no-missing-atx-heading-space": rule11,
"no-missing-label-refs": rule12,
"no-missing-link-fragments": rule13,
"no-multiple-h1": rule14,
"no-reference-like-urls": rule15,
"no-reversed-media-syntax": rule16,
"no-space-in-emphasis": rule17,
"no-unused-definitions": rule18,
"require-alt-text": rule19,
"table-column-count": rule20,
};import rule0 from "../rules/fenced-code-language.js";
import rule1 from "../rules/fenced-code-meta.js";
import rule2 from "../rules/heading-increment.js";
import rule3 from "../rules/no-bare-urls.js";
import rule4 from "../rules/no-duplicate-definitions.js";
import rule5 from "../rules/no-duplicate-headings.js";
import rule6 from "../rules/no-empty-definitions.js";
import rule7 from "../rules/no-empty-images.js";
import rule8 from "../rules/no-empty-links.js";
import rule9 from "../rules/no-html.js";
import rule10 from "../rules/no-invalid-label-refs.js";
import rule11 from "../rules/no-missing-atx-heading-space.js";
import rule12 from "../rules/no-missing-label-refs.js";
import rule13 from "../rules/no-missing-link-fragments.js";
import rule14 from "../rules/no-multiple-h1.js";
import rule15 from "../rules/no-reference-like-urls.js";
import rule16 from "../rules/no-reversed-media-syntax.js";
import rule17 from "../rules/no-space-in-emphasis.js";
import rule18 from "../rules/no-unused-definitions.js";
import rule19 from "../rules/require-alt-text.js";
import rule20 from "../rules/table-column-count.js";
export default {
"fenced-code-language": /** @type {{meta: typeof rule0.meta; create: (context: unknown) => any}} */ (rule0),
"fenced-code-meta": /** @type {{meta: typeof rule1.meta; create: (context: unknown) => any}} */ (rule1),
"heading-increment": /** @type {{meta: typeof rule2.meta; create: (context: unknown) => any}} */ (rule2),
"no-bare-urls": /** @type {{meta: typeof rule3.meta; create: (context: unknown) => any}} */ (rule3),
"no-duplicate-definitions": /** @type {{meta: typeof rule4.meta; create: (context: unknown) => any}} */ (rule4),
"no-duplicate-headings": /** @type {{meta: typeof rule5.meta; create: (context: unknown) => any}} */ (rule5),
"no-empty-definitions": /** @type {{meta: typeof rule6.meta; create: (context: unknown) => any}} */ (rule6),
"no-empty-images": /** @type {{meta: typeof rule7.meta; create: (context: unknown) => any}} */ (rule7),
"no-empty-links": /** @type {{meta: typeof rule8.meta; create: (context: unknown) => any}} */ (rule8),
"no-html": /** @type {{meta: typeof rule9.meta; create: (context: unknown) => any}} */ (rule9),
"no-invalid-label-refs": /** @type {{meta: typeof rule10.meta; create: (context: unknown) => any}} */ (rule10),
"no-missing-atx-heading-space": /** @type {{meta: typeof rule11.meta; create: (context: unknown) => any}} */ (rule11),
"no-missing-label-refs": /** @type {{meta: typeof rule12.meta; create: (context: unknown) => any}} */ (rule12),
"no-missing-link-fragments": /** @type {{meta: typeof rule13.meta; create: (context: unknown) => any}} */ (rule13),
"no-multiple-h1": /** @type {{meta: typeof rule14.meta; create: (context: unknown) => any}} */ (rule14),
"no-reference-like-urls": /** @type {{meta: typeof rule15.meta; create: (context: unknown) => any}} */ (rule15),
"no-reversed-media-syntax": /** @type {{meta: typeof rule16.meta; create: (context: unknown) => any}} */ (rule16),
"no-space-in-emphasis": /** @type {{meta: typeof rule17.meta; create: (context: unknown) => any}} */ (rule17),
"no-unused-definitions": /** @type {{meta: typeof rule18.meta; create: (context: unknown) => any}} */ (rule18),
"require-alt-text": /** @type {{meta: typeof rule19.meta; create: (context: unknown) => any}} */ (rule19),
"table-column-count": /** @type {{meta: typeof rule20.meta; create: (context: unknown) => any}} */ (rule20),
};This type casting made TypeScript happy, since the type incompatibility issue came from the The return type of Also, I changed the following from the first version to the second version, since using /** @type {FencedCodeMetaRuleDefinition} */
export default {
// ...export default /** @satisfies {FencedCodeLanguageRuleDefinition} */ ({
// ...Now, the |
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
This PR fixes a type compatibility issue that makes Markdown plugin v8 incompatible with ESLint v9.39.x, which is now in maintenance mode.
Cause of the type incompatibility problem
Currently, the incompatibility comes from the
export { rules };statement indist/index.d.ts(line 17).This happens because
rulesis exposed such asRecord<RuleId, MarkdownRuleDefinition>, andMarkdownRuleDefinitiondepends onCustomRuleDefinitionType,CustomRuleTypeDefinitions, andCustomRuleVisitorWithExit. Those types were changed as part of ESLint v10’s breaking changes, so some type signatures no longer match, making Markdown plugin v8 incompatible with ESLint v9.39.x.Interestingly, if we avoid coercing the rule types to
MarkdownRuleDefinitionand keep them in their original object form instead, there are no type errors. That is because the rule objects themselves have not changed between versions.I tried a number of different approaches to preserve the original rule object form while still keeping the benefits of type restrictions. In the end, the simplest approach was to coerce the built
src/build/rules.jsobject toRecord<RuleId, any>.This lets us keep type restrictions when writing the code, while preventing end users from running into type incompatibilities caused by major ESLint version bumps.
Potential downsides of this change
Since the user-facing rule type changes from
Record<RuleId, MarkdownRuleDefinition>toRecord<RuleId, any>, directly accessing rule metadata through the index file becomes less strictly typed.However, because directly accessing
rulesis considered an unstable API and is not treated as a breaking change under our policy, this seems acceptable.Ref: https://github.com/eslint/eslint/blob/main/lib/unsupported-api.js
Compatibility with ESLint versions earlier than v9.39.0?
For ESLint versions earlier than v9.39.0, type errors occur not only with
MarkdownRuleDefinitionbut also in other parts, such asprocessor.I'm not sure there is an easy way to make this work with ESLint < v9.39.0. If there is, I'd be happy to follow it.
What changes did you make? (Give an overview)
ci.yml,types.test.ts, andREADME.mdUpdated tests to ensure type compatibility with ESLint v9.39.0, v9.39.x, and v10.x.
src/index.js, andsrc/processor.jsUpdated the types to use the ones from
@eslint/core.Before this change,
@eslint/markdowndepended on types exported from theeslintpackage. This is problematic because those types do not come from a direct dependency; instead, they effectively come from the peer-likeeslintpackage.First,
eslintis currently not declared as a peer dependency, which can cause type errors when@eslint/markdownis used without theeslintpackage. For example, in Code Explorer, we useeslint-linter-browserifyinstead ofeslint.Second, the ESLint types come from the host package and depend on the version installed by the user, making the behavior unpredictable. This is especially problematic because some internal implementation code uses types from
@eslint/core, while types fromeslintare also mixed in.tools/build-rules.jsUpdated the auto-generated
src/build/rules.jsfile to use the typeRecord<RuleId, any>. I believe this will also help avoid type inconsistency errors when updating theRuleDefinitiontype in the future.Related Issues
Ref: eslint/json#213
Is there anything you'd like reviewers to focus on?
N/A