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

chore: bump typescript-eslint to v7 #10271

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JoshuaKGoldberg
Copy link
Contributor

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • [ ] If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • [ ] If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

👋 Hi! I'm coming over from typescript-eslint/typescript-eslint#9141: I'd like to try out the new typescript-eslint v8 version in beta now... but Docusaurus is still on v5. This PR gets you to the latest stable version, v7, without making any other substantial changes.

Test Plan

yarn lint
yarn build

Test links

n/a - should be no functional code changes.

Related issues/PRs

None exist, but if you'd like I'm happy to file any issues you'd want! ❤️

@@ -379,7 +379,6 @@ module.exports = {
// function placeholder params are always ignored, and any other unused
// locals must be justified with a disable comment.
'@typescript-eslint/no-unused-vars': [ERROR, {ignoreRestSiblings: true}],
'@typescript-eslint/prefer-optional-chain': ERROR,
Copy link
Contributor Author

@JoshuaKGoldberg JoshuaKGoldberg Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As much as I love this rule, it needs linting with type information to be bug-free. typescript-eslint/typescript-eslint#6397 changed the rule to require type info in typescript-eslint v6.

I briefly tried adding typed linting in this PR but:

...so my suggestion would be to wait until v8 to use the stabilized parserOptions.projectService.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, we had a weird type checking setup (to make tests type checked), and I don't remember a good way to deal with it.

Copy link

netlify bot commented Jul 5, 2024

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 1430700
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/66888e23d56a830008b1756c
😎 Deploy Preview https://deploy-preview-10271--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Jul 5, 2024

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO Report
/ 🟠 79 🟢 98 🟢 96 🟢 100 Report
/docs/installation 🟠 56 🟢 97 🟢 100 🟢 100 Report
/docs/category/getting-started 🟠 75 🟢 100 🟢 100 🟠 86 Report
/blog 🟠 70 🟢 100 🟢 100 🟠 86 Report
/blog/preparing-your-site-for-docusaurus-v3 🔴 49 🟢 96 🟢 100 🟢 100 Report
/blog/tags/release 🟠 70 🟢 100 🟢 100 🟠 86 Report
/blog/tags 🟠 75 🟢 100 🟢 100 🟠 86 Report

@@ -6,7 +6,7 @@
*/

import {createRule} from '../util';
import type {TSESTree} from '@typescript-eslint/types/dist/ts-estree';
import type {TSESTree} from '@typescript-eslint/types';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package exports were cleaned up in typescript-eslint v6: https://typescript-eslint.io/blog/announcing-typescript-eslint-v6#package-exports

@@ -35,7 +35,6 @@ export default createRule<Options, MessageIds>({
type: 'problem',
docs: {
description: 'enforce using Docusaurus Link component instead of <a> tag',
recommended: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are no longer required in types for rules, and it looks to me that these weren't used anywhere in Docusaurus code.

@JoshuaKGoldberg
Copy link
Contributor Author

Failing because tests run on Node 18, not Node 18.18. I asked in Discord: https://discord.com/channels/398180168688074762/584803742801723424/1258853596338847825

@slorber
Copy link
Collaborator

slorber commented Jul 9, 2024

Thanks

Failing because tests run on Node 18, not Node 18.18. I asked in Discord: discord.com/channels/398180168688074762/584803742801723424/1258853596338847825

Is this the only blocker to this PR?

We still support Node 18.0, but we'll very likely upgrade to Node 20 in the next major

@slorber slorber added the pr: internal This PR does not touch production code, or is not meaningful enough to be in the changelog. label Jul 9, 2024
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] environment, filesystem +5 230 kB ljharb
npm/[email protected] None 0 32.1 kB feross
npm/[email protected] environment Transitive: eval, unsafe +5 1.31 MB evilebottnawi
npm/[email protected] None 0 95.4 kB npm-cli-ops
npm/[email protected] None +1 23.3 kB redonkulus
npm/[email protected] None 0 45 kB ljharb
npm/[email protected] None 0 140 kB 7rulnik
npm/[email protected] None +3 123 kB sindresorhus
npm/[email protected] filesystem Transitive: unsafe +9 5.95 MB sethiii
npm/[email protected] None 0 46.9 kB sokra
npm/[email protected] Transitive: environment, filesystem, network, shell, unsafe +8 7.37 MB evilebottnawi
npm/[email protected] environment, eval Transitive: filesystem, shell +7 3.42 MB fabiosantoscode
npm/[email protected] None 0 84 kB typescript-bot
npm/[email protected] None 0 202 kB sindresorhus
npm/[email protected] None 0 40.6 MB typescript-bot
npm/[email protected] None +3 180 kB wooorm
npm/[email protected] None 0 4.31 kB dougwilson
npm/[email protected] None +1 493 kB garycourt
npm/[email protected] None 0 5.48 kB tootallnate
npm/[email protected] None +2 110 kB wooorm
npm/[email protected] None +5 67.5 kB sindresorhus

🚮 Removed packages: npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: internal This PR does not touch production code, or is not meaningful enough to be in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants