refactor: migrate type declarations to use the @import syntax

[lumirlumir]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request?

Hello,

In this PR, I’ve migrated the type declarations to use the @import syntax and added support for MessageIds.

This change was briefly discussed in #336 (comment).

I believe this migration will help ensure consistent type declarations across plugin repositories.

However, one concern I have is that the type module path for RuleModule and MarkdownRuleDefinition has changed from @eslint/markdown to @eslint/markdown/types. While this path is already used in the CSS and JSON plugins, I’m worried it could introduce a breaking change.

I’d appreciate any suggestions on how to avoid making this a breaking change, if possible.

What changes did you make? (Give an overview)

I’ve migrated the type declarations to use the @import syntax and added support for MessageIds.

Related Issues

ref: #336 (comment)

Is there anything you'd like reviewers to focus on?

Prerequisite

• fix!: remove rollup and extraneous types #383
• fix!: move certain types to import them from @eslint/markdown/types #446

[lumirlumir]

I've followed the script from JSON.

[lumirlumir]

The missing type checking issue reported in #432 has been addressed in this PR, so type checking now works as expected for all files.

[nzakas]

Can we just use the regular names? I think that makes things a lot easier to understand.

Suggested change

	* @import { MarkdownRuleVisitor as MRV, MarkdownRuleDefinition as MRD, MarkdownRuleDefinitionTypeOptions } from "./types.js";
	* @import { MarkdownRuleVisitor , MarkdownRuleDefinition, MarkdownRuleDefinitionTypeOptions } from "./types.js";

[lumirlumir]

I initially tried using the original name, but without an alias, it results in a duplicate type definitions error when running the tsc command, since the same name is declared again in the following lines using the @typedef syntax.

(This redeclaration is intentional, for the reason I mentioned here.)

However, if we decide to remove these types from the @eslint/markdown export path and move them to the @eslint/markdown/types path, then these lines can be safely removed. (ref: here)

[fasttime]

Maybe instead of renaming the imported types we could import the whole types module as a namespace to avoid the naming collision? I.e:

/**
 * @import { Linter } from "eslint";
 * @import * as Types from "./types.js";
 * @typedef {Linter.RulesRecord} RulesRecord
 * @typedef {Types.MarkdownRuleDefinition} RuleModule
 * @typedef {Types.MarkdownRuleVisitor} MarkdownRuleVisitor
 */

And similarly for the MarkdownRuleDefinition typedef below.

[lumirlumir]

Awesome, it works 👍

I've added a new commit a001891

[nzakas]

Then I don't think we need these.

[lumirlumir]

This is because the type tests in types.test.js currently depend on these type definitions.
So, removing them in this PR would introduce a breaking change:

markdown/tests/types/types.test.ts

Lines 1 to 6 in 47f06b7

	import markdown, {
	MarkdownSourceCode,
	MarkdownRuleDefinition,
	MarkdownRuleVisitor,
	type RuleModule,
	} from "@eslint/markdown";

Personally, I think it would be better to keep this PR non-breaking and create a separate PR to remove these types and move them under the @eslint/markdown/types path when importing.

Both the JSON and CSS plugins follow a similar approach, so it would make sense to organize the types this way:

• JSON plugin:
  https://github.com/eslint/json/blob/3f754f7f5f8e182ce88fc65a909b58a0116d04ac/tests/types/types.test.ts#L3-L7
• CSS plugin:
  https://github.com/eslint/css/blob/211bf21f3c72530e60105a4f89c708bcbb00fc82/tests/types/types.test.ts#L55

If it’s acceptable, I can open a separate PR with the breaking changes by tomorrow.

[nzakas]

Sounds good. Because the next release will be a major release, this is the time to get all the breaking changes in. 👍

[lumirlumir]

I've made a separate PR #446 👍

[lumirlumir]

Suggested change

	* @typedef {Types.MarkdownRuleDefinition<Options>} MarkdownRuleDefinition<Options>
	* @typedef {Types.MarkdownRuleDefinition<Options>} MarkdownRuleDefinition<Options>

This line should be separated from the typedefs above; otherwise, it causes a type error like the one below:

[fasttime]

LGTM, thanks! Leaving open for @nzakas to verify before merging.

[snitin315]

LGTM

[JoshuaKGoldberg]

@eslint/markdown/types ... already used in the CSS and JSON plugins

👋 I'm catching up on this PR now and there's a lot of context. But I can't find where the original decision on that was made. Was there a driving reason to put types in the specific /types export? This is pretty unusual.

[fasttime]

@eslint/markdown/types ... already used in the CSS and JSON plugins

👋 I'm catching up on this PR now and there's a lot of context. But I can't find where the original decision on that was made. Was there a driving reason to put types in the specific /types export? This is pretty unusual.

@JoshuaKGoldberg There was a discussion in eslint/json#84. Do you know a better solution? I have to admit I haven't seen our approach used anywhere else.

[JoshuaKGoldberg]

Ah, I'd missed that PR. Darn.

Generally, projects try to in order of preference:

1. Not auto-generate types, so they can be directly exported from the root export
2. Have type auto-generation always run before publish, so they can still be exported from the root export

From eslint/json#82 (comment):

there's no easy way to just say "export this type"

Just to confirm, is the issue that index.js needs to export types from its sibling types.ts - and there's no way in TypeScript's JSDoc support to "re-export" a type in a JS file?

I haven't dug too deep, but the first thing that comes to mind is injecting an \nexport type * from "./types.d.ts"; at the bottom of the dist/*/index.d.ts files. That would do it in a pinch.

[lumirlumir]

@fasttime @JoshuaKGoldberg

Hi everyone, is there anything I need to do on my side? If so, I’d be happy to take a look.

[JoshuaKGoldberg]

I don't know what the right next step is 😅. #453 is a reference for the alternate strategy of adding a re-export line.

I also asked for help in the TypeScript Discord to see if there's a better way: https://discord.com/channels/508357248330760243/1389274620212809829

Edit: and Bluesky, https://bsky.app/profile/did:plc:hwtki3j7oghodc7h6gqnrtro/post/3lsu2bsv4gs2w

[lumirlumir]

@JoshuaKGoldberg Thanks for the references!

I’ve explored some alternatives, but I think #453 is the best solution for now 👍

---

@nzakas I’d appreciate it if you could take a final look when you have time 😄

(FYI, I’ve resolved the merge conflicts, so the previous review has gone stale)

[nzakas]

Given that we've already followed the /types approach in @eslint/json, I think it's fine to keep it here. We can always revisit later if we decide there's a cleaner approach and then apply that all of the plugins.