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

Types: Allow passing JsonAST nodes directly to ESLint context APIs #198

Open
JoshuaKGoldberg opened this issue Jan 20, 2024 · 3 comments
Open
Labels
enhancement New feature or request

Comments

@JoshuaKGoldberg
Copy link
Contributor

Description

Following #184 -> #186: I'm still finding that I need to do some massaging of TypeScript types. Also filed in JoshuaKGoldberg/eslint-plugin-package-json#125.

The current state of types for AST nodes in eslint-plugin-package-json is that we've completely swapped out the built-in ESLint ones. Nodes are instead typed using import type { AST as JsonAST } from "jsonc-eslint-parser"; -> node: JsonAST.JSONProperty and the like.

Unfortunately, ESLint's APIs still expect ESTree nodes. So we end up with assertions any time we need to pass the nodes to ESLint's APIs:

context.report({ 
  messageId: "mismatched", 
  node: node as unknown as ESTree.Node, 
});

This isn't ideal. One workaround done by eslint-plugin-jsonc is to [instead pass loc: node.loc](https://github.com/ota-meshi/eslint-plugin-jsonc/blob/bb0cc408c928ef12952d140add87f3c2b8a2eb40/lib/rules/no-bigint-literals.ts#L26-L27). But I'd ideally like to be able to just pass the node` directly.

Is this something we can solve at the parser level in jsonc-eslint-parser? Slash, maybe we should modify @types/eslint in some way to make the context generic (per JoshuaKGoldberg/eslint-plugin-package-json#125 additional info)?

@ota-meshi
Copy link
Owner

Thank you for posting this issue.

Hmm... In my opinion, the best option is to change @types/eslint to accept ESTree.Node | {type: string, loc: SourceLocation} as a node.
What do you think?

@JoshuaKGoldberg
Copy link
Contributor Author

Oh! I should have thought of that 🤦. Yes, that makes sense. +1 😄

@JounQin
Copy link
Collaborator

JounQin commented Jan 21, 2024

Related to DefinitelyTyped/DefinitelyTyped#68232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants