8000 feat: support stringly-typed extends by kirkwaiblinger · Pull Request #10973 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]

Skip to content

feat: support stringly-typed extends #10973

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

Merged

Conversation

kirkwaiblinger
Copy link
Member
@kirkwaiblinger kirkwaiblinger commented Mar 19, 2025

PR Checklist

Overview

Previously,

  • eslintrc configs were stored at packages/eslint-plugin/src/configs, and were exposed via the plugin.configs field
  • flat configs were stored at packages/typescript-eslint/src/configs, and were exposed via the tseslint.configs field.
  • a typescript-eslint parser object was created in the typescript-eslint package, is used inside the flat configs, and was exposed via tseslint.parser.
  • eslintrc configs are stringly typed, so there was no need for exporting configs or parser objects from eslint-plugin (only "exception" was a little use-at-your-own-risk config generation function - this was just to share code, not for any runtime reason).

Now,

  • eslintrc configs are stored at packages/eslint-plugin/src/configs/eslintrc.
  • flat configs are stored at packages/eslint-plugin/src/configs/flat.
  • The plugin.configs object contains all the same eslintrc configs and all the flat configs, prefixed with 'flat/. Conceptually,
    const plugin = {
       configs: {
           strict: eslintrcStrict,
           'flat/strict': flatStrict,
           // likewise for the rest of the configs...
       }
       // rest of the plugin fields...
    };
    Only the eslintrc configs are acknowledged in the publicly exported types. Therefore, this will be invisible to users in the editor.
  • the tseslint.configs does not use the flat/ prefixes. It only exports eslint v9 flat configs, just like it always has.
  • Because the flat configs need a parser object, the eslint-plugin package now creates the parser object.
  • In order for relevant objects in eslint-plugin to be === to their counterparts in typescript-eslint at runtime, the flatConfigs and parser are now exported from eslint-plugin via a new use-at-your-own-risk export in order to be re-exported by typescript-eslint.

In summary, these changes ensure:

const assert = require('node:assert');
const { defineConfig } = require('eslint/config');
const tseslint = require('typescript-eslint');
const tseslintPlugin = require('@typescript-eslint/eslint-plugin');

assert(tseslint.plugin === tseslintPlugin);
assert(tseslint.configs.strict === tseslintPlugin.configs['flat/strict']);
assert(tseslint.parser === tseslintPlugin.configs['flat/strict'][0].parser);

// doesn't throw
defineConfig({
   extends: ['tseslint/strict'],
   plugins: { tseslint: tseslintPlugin },
});

// BUT
assert(tseslint.configs !== tseslintPlugin.configs);

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @kirkwaiblinger!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link
netlify bot commented Mar 19, 2025

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 39228c2
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/67f74a71e91869000893c5e6
😎 Deploy Preview https://deploy-preview-10973--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 98 (🟢 up 10 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

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

Copy link
nx-cloud bot commented Mar 19, 2025

View your CI Pipeline Execution ↗ for commit 39228c2.

Command Status Duration Result
nx run-many --target=build --exclude website --... ✅ Succeeded 6s View ↗
nx run-many --target=clean ✅ Succeeded 10s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-10 15:57:17 UTC

@@ -148,7 +148,8 @@
"react-split-pane@^0.1.92": "patch:react-split-pane@npm%3A0.1.92#./.yarn/patches/react-split-pane-npm-0.1.92-93dbf51dff.patch",
"tmp": "0.2.1",
"tsx": "^4.7.2",
"typescript": "5.8.2"
"typescript": "5.8.2",
"@eslint/config-helpers": "^0.2.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

This manual resolution is required pending release of the next version of eslint (eslint/eslint#19533 has been merged but not yet released)

Copy link
Member

Choose a reason for hiding this comment

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

We're clear to remove this now, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

well, on main we still have

├─ eslint@npm:9.22.0
│  └─ @eslint/config-helpers@npm:0.1.0 (via npm:^0.1.0)
│
└─ eslint@npm:9.22.0 [43684]
   └─ @eslint/config-helpers@npm:0.1.0 (via npm:^0.1.0)

so we need either this or to upgrade eslint past 9.23

Copy link
Member Author
@kirkwaiblinger kirkwaiblinger Apr 10, 2025

Choose a reason for hiding this comment

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

(I've implemented the latter, updating our eslint dependency) I've kept the resolutions since I had some CI failures that I had a hard time debugging when I tried upgrading the eslint version.

Copy link
Member Author

Choose a reason for hiding this comment

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

but yeah once main has eslint >= 9.23 we can remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Fun. I just clicked the checkmark in #6687 to start the Renovate PR.

@kirkwaiblinger kirkwaiblinger added the blocked by external API Blocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API label Mar 20, 2025
Copy link
codecov bot commented Mar 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.08%. Comparing base (23c5aa7) to head (39228c2).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10973      +/-   ##
==========================================
+ Coverage   88.01%   88.08%   +0.07%     
==========================================
  Files         470      497      +27     
  Lines       16805    16901      +96     
  Branches     4746     4746              
==========================================
+ Hits        14791    14888      +97     
  Misses       1667     1667              
+ Partials      347      346       -1     
Flag Coverage Δ
unittest 88.08% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...slint-plugin/src/configs/eslint-recommended-raw.ts 100.00% <ø> (+25.00%) ⬆️
packages/eslint-plugin/src/configs/eslintrc/all.ts 100.00% <ø> (ø)
...ackages/eslint-plugin/src/configs/eslintrc/base.ts 100.00% <ø> (ø)
...lugin/src/configs/eslintrc/disable-type-checked.ts 100.00% <ø> (ø)
...-plugin/src/configs/eslintrc/eslint-recommended.ts 100.00% <100.00%> (ø)
.../configs/eslintrc/recommended-type-checked-only.ts 100.00% <ø> (ø)
...n/src/configs/eslintrc/recommended-type-checked.ts 100.00% <ø> (ø)
.../eslint-plugin/src/configs/eslintrc/recommended.ts 100.00% <ø> (ø)
...n/src/configs/eslintrc/strict-type-checked-only.ts 100.00% <ø> (ø)
...plugin/src/configs/eslintrc/strict-type-checked.ts 100.00% <ø> (ø)
... and 18 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kirkwaiblinger kirkwaiblinger removed the blocked by external API Blocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API label Mar 22, 2025
@kirkwaiblinger kirkwaiblinger marked this pull request as ready for review March 22, 2025 19:41
Copy link
Member
@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM, really clean swap-out. Nicely done!

Just requesting changes on a passing build & removing an unused var.

Anne Hathaway clapping at an awards show, smiling happily and tearing up

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Mar 31, 2025
@kirkwaiblinger kirkwaiblinger added the awaiting response Issues waiting for a reply from the OP or another party label Apr 9, 2025
kirkwaiblinger and others added 3 commits April 9, 2025 19:18
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
bradzacher
bradzacher previously approved these changes Apr 10, 2025
import type { TSESLint } from '@typescript-eslint/utils';
import type { FlatConfig, Linter } from '@typescript-eslint/utils/ts-eslint';

import * as parserBase from '@typescript-eslint/parser';
Copy link
Member

Choose a reason for hiding this comment

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

I hate that we have to add an explicit dependency on our parser inside our plugin now.
But it is what it is.

defineConfig({
extends: ['ts/flat/strict'],
plugins: {
// @ts-expect-error -- types aren't compatible.
Copy link
Member

Choose a reason for hiding this comment

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

side note:
it's been on my todo list for a while -- we need to just bite the bullet and make them compatible.

@github-actions github-actions bot removed 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge awaiting response Issues waiting for a reply from the OP or another party labels Apr 10, 2025
Copy link
Member
@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Two young cats humorously jumping around and one flipping into a trashcan to get to a cat toy their owner is moving around

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Apr 10, 2025
Copy link
Member
@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

lgtm15.jpg

@bradzacher bradzacher merged commit 69e2f6c into typescript-eslint:main Apr 10, 2025
60 of 62 checks passed
@kirkwaiblinger kirkwaiblinger deleted the add-flat-configs-to-plugin branch April 11, 2025 03:51
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: support ESLint's new defineConfig string config references
4 participants
0