fix: update TypeScript to v6 and tighten the return type of getParent#637
fix: update TypeScript to v6 and tighten the return type of getParent#637
getParent#637Conversation
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
…or/update-typescript-to-v6
getParent.
getParent.getParent
getParentgetParent
getParentgetParent
…or/update-typescript-to-v6
| "verbatimModuleSyntax": true, | ||
| "erasableSyntaxOnly": true |
There was a problem hiding this comment.
To prevent the same issue as in eslint/json#88, I've also set the verbatimModuleSyntax and erasableSyntaxOnly options to true, as in the JSON type test.
https://github.com/eslint/json/blob/main/tests/types/tsconfig.json#L12-L13
| export default defineConfig([ | ||
| globalIgnores( | ||
| ["**/*.js", "**/.cjs", "**/.mjs"], | ||
| ["**/*.js", "**/.cjs", "**/.mjs", "tests/fixtures/"], |
There was a problem hiding this comment.
This addition prevents the warning that appears when I run npm run lint:
Reference:
Line 27 in e2396bb
| if (node.children) { | ||
| node.children.forEach(child => { | ||
| visit(child, node); | ||
| if ("children" in node) { |
There was a problem hiding this comment.
In the Markdown AST (Mdast), if a node has children they are always represented as an array (even when empty), so I think refactoring node.children to "children" in node is fine here.
| * Returns the parent of the given node. | ||
| * @param {Node} node The node to get the parent of. | ||
| * @returns {Node|undefined} The parent of the node. | ||
| * @returns {Parent|undefined} The parent of the node. |
There was a problem hiding this comment.
This is why the PR is marked as fix.
If the value is not undefined, the node is added to the private weakmap #parents only after the "children" in node check on line 331. In the Markdown AST (Mdast), a node with a children property always extends the Parent type, so using Parent instead of Node seems more accurate here.
| "rootDir": "./src", | ||
| "strictNullChecks": false, | ||
| "useUnknownInCatchVariables": false, | ||
| "types": ["mdast", "unist"] |
There was a problem hiding this comment.
Why are these types packages included globally?
There was a problem hiding this comment.
Thanks for pointing that out. I misunderstood how the types field works.
I've now realized how it works and updated it in 6399827.
ff42108 to
6399827
Compare
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
Note
This PR follows the same strategy used in eslint/json#224 and eslint/json#230.
This PR updates TypeScript to v6.
The following two changes mainly affected us during the upgrade to TypeScript v6.
strictmode is now the default.https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/#simple-default-changes
rootdirnow defaults to.https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/#rootdir-now-defaults-to-.
What changes did you make? (Give an overview)
I've tried to retain strict mode behavior as much as possible and only disabled it where it produced too many errors.
I've written more detailed descriptions as comments where necessary.
Related Issues
Refs: eslint/json#224, eslint/json#230
Is there anything you'd like reviewers to focus on?
N/A