Repository navigation
fix: type compatibility issue between JSON v2 and ESLint v9 - #248
Conversation
| let keys; | ||
|
|
||
| return { | ||
| return /** @type {JSONRuleVisitor} */ ({ |
There was a problem hiding this comment.
Unlike the CSS implementation, if we don’t type-cast the return type with JSONRuleVisitor, a portable type error occurs:
The inferred type of 'create' cannot be named without a reference to 'MemberNode' from '../../node_modules/@humanwhocodes/momoa/dist/typedefs.js'. This is likely not portable. A type annotation is necessary.This was also why only the JSON plugin used the following snippet:
const rule = { /* ... */ };
export default rule;Instead of:
export default { /* ... */ };I’ve tried to find an easy way to resolve it, but I couldn’t find one we could handle on our own.
I think this issue lies with @humanwhocodes/momoa, since the package still uses a dual CJS/ESM setup and type resolution seems to have broken at some point.
Sending a PR to @humanwhocodes/momoa to make it ESM-only may resolve the type issue, since the portable error seems to come from typedefs.ts not being exported directly from the main entry point ./dist/momoa.js, but instead being exported indirectly.
There was a problem hiding this comment.
I'm not sure how to export dist/typedefs.ts directly from dist/momoa.js. Do you have an idea?
There was a problem hiding this comment.
What I found is that in the distributed type declaration file dist/momoa.d.ts from @humanwhocodes/momoa, if we remove export type AnyNode = import("./typedefs.js").AnyNode; and instead explicitly import and export the types (for example, import type { AnyNode } from "./typedefs.js"; and export type { AnyNode } from "./typedefs.js";), the portable type error is resolved.
Here is a minimal patch I applied directly in node_modules, and it worked as expected:
Patched File
import type {
AnyNode,
JSONValue,
Token,
TokenizeOptions,
NodeParts,
DocumentNode,
StringNode,
NumberNode,
BooleanNode,
MemberNode,
ObjectNode,
ElementNode,
ArrayNode,
NullNode,
ValueNode,
IdentifierNode,
NaNNode,
InfinityNode,
Sign,
Node,
ParseOptions,
TraversalPhase,
} from "./typedefs.js";
export type {
AnyNode,
JSONValue,
TokenType,
Location,
Token,
Range,
TokenizeOptions,
LocationRange,
NodeParts,
DocumentNode,
StringNode,
NumberNode,
BooleanNode,
MemberNode,
ObjectNode,
ElementNode,
ArrayNode,
NullNode,
ValueNode,
IdentifierNode,
NaNNode,
InfinityNode,
Sign,
Node,
Mode,
ParseOptions,
TraversalPhase,
} from "./typedefs.js";
export type FilterPredicate = (item: {
node: Node;
parent?: Node;
phase: TraversalPhase;
}, index: number, array: Array<{
node: Node;
parent?: Node;
phase: TraversalPhase;
}>) => boolean;
export type TraversalVisitor = {
enter?: (node: Node, parent?: Node) => void;
exit?: (node: Node, parent?: Node) => void;
};
/**
* @fileoverview Evaluator for Momoa AST.
* @author Nicholas C. Zakas
*/
/** @typedef {import("./typedefs.js").AnyNode} AnyNode */
/** @typedef {import("./typedefs.js").JSONValue} JSONValue */
/**
* Evaluates a Momoa AST node into a JavaScript value.
* @param {AnyNode} node The node to interpet.
* @returns {JSONValue} The JavaScript value for the node.
*/
export function evaluate(node: AnyNode): JSONValue;
/**
* @callback FilterPredicate
* @param {{node: Node, parent?: Node, phase: TraversalPhase}} item
* @param {number} index
* @param {Array<{node: Node, parent?: Node, phase: TraversalPhase}>} array
* @returns {boolean}
*/
/**
* Creates an iterator over the given AST.
* @param {Node} root The root AST node to traverse.
* @param {FilterPredicate} [filter] A filter function to determine which steps to
* return;
* @returns {IterableIterator<{node: Node, parent?: Node, phase: TraversalPhase}>} An iterator over the AST.
*/
export function iterator(root: Node, filter?: FilterPredicate): IterableIterator<{
node: Node;
parent?: Node;
phase: TraversalPhase;
}>;
/**
*
* @param {string} text The text to parse.
* @param {ParseOptions} [options] The options object.
* @returns {DocumentNode} The AST representing the parsed JSON.
* @throws {Error} When there is a parsing error.
*/
export function parse(text: string, options?: ParseOptions): DocumentNode;
/**
* Converts a Momoa AST back into a JSON string.
* @param {AnyNode} node The node to print.
* @param {Object} options Options for the print.
* @param {number} [options.indent=0] The number of spaces to indent each line. If
* greater than 0, then newlines and indents will be added to output.
* @returns {string} The JSON representation of the AST.
*/
export function print(node: AnyNode, { indent }?: {
indent?: number;
}): string;
/**
* Creates an iterator over the tokens representing the source text.
* @param {string} text The source text to tokenize.
* @param {TokenizeOptions} [options] Options for doing the tokenization.
* @returns {Array<Token>} An iterator over the tokens.
*/
export function tokenize(text: string, options?: TokenizeOptions): Array<Token>;
/**
* Traverses an AST from the given node.
* @param {Node} root The node to traverse from
* @param {TraversalVisitor} visitor An object with an `enter` and `exit` method.
*/
export function traverse(root: Node, visitor: TraversalVisitor): void;
export namespace types {
/**
* Creates a document node.
* @param {ValueNode} body The body of the document.
* @param {NodeParts} parts Additional properties for the node.
* @returns {DocumentNode} The document node.
*/
export function document(body: ValueNode, parts?: NodeParts): DocumentNode;
/**
* Creates a string node.
* @param {string} value The value for the string.
* @param {NodeParts} parts Additional properties for the node.
* @returns {StringNode} The string node.
*/
export function string(value: string, parts?: NodeParts): StringNode;
/**
* Creates a number node.
* @param {number} value The value for the number.
* @param {NodeParts} parts Additional properties for the node.
* @returns {NumberNode} The number node.
*/
export function number(value: number, parts?: NodeParts): NumberNode;
/**
* Creates a boolean node.
* @param {boolean} value The value for the boolean.
* @param {NodeParts} parts Additional properties for the node.
* @returns {BooleanNode} The boolean node.
*/
export function boolean(value: boolean, parts?: NodeParts): BooleanNode;
/**
* Creates a null node.
* @param {NodeParts} parts Additional properties for the node.
* @returns {NullNode} The null node.
*/
function _null(parts?: NodeParts): NullNode;
export { _null as null };
/**
* Creates an array node.
* @param {Array<ElementNode>} elements The elements to add.
* @param {NodeParts} parts Additional properties for the node.
* @returns {ArrayNode} The array node.
*/
export function array(elements: Array<ElementNode>, parts?: NodeParts): ArrayNode;
/**
* Creates an element node.
* @param {ValueNode} value The value for the element.
* @param {NodeParts} parts Additional properties for the node.
* @returns {ElementNode} The element node.
*/
export function element(value: ValueNode, parts?: NodeParts): ElementNode;
/**
* Creates an object node.
* @param {Array<MemberNode>} members The members to add.
* @param {NodeParts} parts Additional properties for the node.
* @returns {ObjectNode} The object node.
*/
export function object(members: Array<MemberNode>, parts?: NodeParts): ObjectNode;
/**
* Creates a member node.
* @param {StringNode|IdentifierNode} name The name for the member.
* @param {ValueNode} value The value for the member.
* @param {NodeParts} parts Additional properties for the node.
* @returns {MemberNode} The member node.
*/
export function member(name: StringNode | IdentifierNode, value: ValueNode, parts?: NodeParts): MemberNode;
/**
* Creates an identifier node.
* @param {string} name The name for the identifier.
* @param {NodeParts} parts Additional properties for the node.
* @returns {IdentifierNode} The identifier node.
*/
export function identifier(name: string, parts?: NodeParts): IdentifierNode;
/**
* Creates a NaN node.
* @param {Sign} sign The sign for the Infinity.
* @param {NodeParts} parts Additional properties for the node.
* @returns {NaNNode} The NaN node.
*/
export function nan(sign?: Sign, parts?: NodeParts): NaNNode;
/**
* Creates an Infinity node.
* @param {Sign} sign The sign for the Infinity.
* @param {NodeParts} parts Additional properties for the node.
* @returns {InfinityNode} The Infinity node.
*/
export function infinity(sign?: Sign, parts?: NodeParts): InfinityNode;
}
/**
* @fileoverview Traversal approaches for Momoa JSON AST.
* @author Nicholas C. Zakas
*/
/** @typedef {import("./typedefs.js").TraversalPhase} TraversalPhase */
/**
* @typedef {Object} TraversalVisitor
* @property {(node: Node, parent?: Node) => void} [enter]
* @property {(node: Node, parent?: Node) => void} [exit]
*/
declare const childKeys: Map<string, string[]>;
export { childKeys as visitorKeys };
But the problem is that, in the current state of @humanwhocodes/momoa, the build process depends on rollup, similar to how we do it in the rewrite repository, so there doesn’t seem to be an easy way to resolve this issue.
One option would be to jump through multiple hoops to produce the type declaration file we want when building declarations in @humanwhocodes/momoa. But based on eslint/rewrite#403 (comment), I’m not sure that would be the preferred approach.
Another option would be to bump @humanwhocodes/momoa to v4 and make it ESM-only with an updated Node.js support range and apply the same feature from eslint/rewrite#403.
I think resolving this issue clearly will require some effort. But as a practical option for now, I believe we could track the JSONRuleVisitor type cast in a separate issue and remove it once this is resolved in @humanwhocodes/momoa. What do you think about this direction?
There was a problem hiding this comment.
I see. So using type export statements instead of import()-based type references avoids the compiler error when TypeScript can't infer a portable return type. That still feels a bit surprising, because the import() syntax doesn't seem problematic in the other cases, and it also appears to work fine when the referenced module isn't under node_modules.
I don't think we need to track the JSONRuleVisitor cast separately, since ideally we'd remove this workaround altogether at some point. The type cast should be good enough until then.
As for updating the @humanwhocodes/momoa types, I'm not sure either. My current understanding is that this is related to how the types are declared in JSDoc (with /** @typedef {import("./typedefs.js").MemberNode} MemberNode */), rather than something specific to CommonJS transpilation or Rollup. Maybe you could open a separate issue to discuss the change you have in mind in @humanwhocodes/momoa sepecifically.
There was a problem hiding this comment.
Thanks, I've understood the situation. I opened a separate issue in @humanwhocodes/momoa to discuss what I have in mind: humanwhocodes/momoa#236.
Is there anything else in particular that I need to update?
|
Friendly ping @fasttime |
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
This PR fixes an issue where JSON v2 and ESLint v9 are not type-compatible, as mentioned in #213.
What changes did you make? (Give an overview)
This PR follows the same strategy determined in eslint/markdown#648. That said, a few parts differ from the Markdown implementation, as follows:
processor, and the blocker for compatibility with lower ESLint v9 versions was thatprocessor. Since CSS does not have aprocessorconfigured, broader type compatibility can be achieved.README.mdthat type compatibility is ensured for v9.15.0 or above, since the current type compatibility range is the same as the one we already support.Related Issues
Refs: eslint/markdown#648, eslint/css#473
Fixes: #213
Is there anything you'd like reviewers to focus on?
N/A