Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

kirkwaiblinger
Copy link
Member

@kirkwaiblingerkirkwaiblinger commentedMar 19, 2025
edited
Loading

PR Checklist

Overview

Previously,

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

Now,

  • eslintrc configs are stored atpackages/eslint-plugin/src/configs/eslintrc.
  • flat configs are stored atpackages/eslint-plugin/src/configs/flat.
  • Theplugin.configs object contains all the same eslintrc configsand all the flat configs, prefixed with'flat/. Conceptually,
    constplugin={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.
  • thetseslint.configsdoes not use theflat/ prefixes. Itonly 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 ineslint-plugin to be=== to their counterparts intypescript-eslint at runtime, theflatConfigs andparser are now exported fromeslint-plugin via a newuse-at-your-own-risk export in order to be re-exported bytypescript-eslint.

In summary, these changes ensure:

constassert=require('node:assert');const{ defineConfig}=require('eslint/config');consttseslint=require('typescript-eslint');consttseslintPlugin=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 throwdefineConfig({extends:['tseslint/strict'],plugins:{tseslint:tseslintPlugin},});// BUTassert(tseslint.configs!==tseslintPlugin.configs);

niklaswimmer reacted with heart emoji
@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 onhttps://opencollective.com/typescript-eslint.

@netlifyNetlify
Copy link

netlifybot commentedMar 19, 2025
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit39228c2
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/67f74a71e91869000893c5e6
😎 Deploy Previewhttps://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 yourNetlify site configuration.

@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedMar 19, 2025
edited
Loading

View yourCI Pipeline Execution ↗ for commit39228c2.

CommandStatusDurationResult
nx run-many --target=build --exclude website --...✅ Succeeded6sView ↗
nx run-many --target=clean✅ Succeeded10sView ↗

☁️Nx Cloud last updated this comment at2025-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
MemberAuthor

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)

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
MemberAuthor

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 past9.23

Copy link
MemberAuthor

@kirkwaiblingerkirkwaiblingerApr 10, 2025
edited
Loading

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 theresolutions since I had some CI failures that I had a hard time debugging when I tried upgrading the eslint version.

Copy link
MemberAuthor

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.

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 reacted with thumbs up emoji
@kirkwaiblingerkirkwaiblinger added the blocked by external APIBlocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API labelMar 20, 2025
@codecovCodecov
Copy link

codecovbot commentedMar 20, 2025
edited
Loading

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
FlagCoverage Δ
unittest88.08% <100.00%> (+0.07%)⬆️

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

Files with missing linesCoverage Δ
...slint-plugin/src/configs/eslint-recommended-raw.ts100.00% <ø> (+25.00%)⬆️
packages/eslint-plugin/src/configs/eslintrc/all.ts100.00% <ø> (ø)
...ackages/eslint-plugin/src/configs/eslintrc/base.ts100.00% <ø> (ø)
...lugin/src/configs/eslintrc/disable-type-checked.ts100.00% <ø> (ø)
...-plugin/src/configs/eslintrc/eslint-recommended.ts100.00% <100.00%> (ø)
.../configs/eslintrc/recommended-type-checked-only.ts100.00% <ø> (ø)
...n/src/configs/eslintrc/recommended-type-checked.ts100.00% <ø> (ø)
.../eslint-plugin/src/configs/eslintrc/recommended.ts100.00% <ø> (ø)
...n/src/configs/eslintrc/strict-type-checked-only.ts100.00% <ø> (ø)
...plugin/src/configs/eslintrc/strict-type-checked.ts100.00% <ø> (ø)
... and18 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.

@kirkwaiblingerkirkwaiblinger removed the blocked by external APIBlocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API labelMar 22, 2025
@kirkwaiblingerkirkwaiblinger marked this pull request as ready for reviewMarch 22, 2025 19:41
JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesMar 31, 2025
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg 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

kirkwaiblinger reacted with heart emoji
@JoshuaKGoldbergJoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelMar 31, 2025
@kirkwaiblingerkirkwaiblinger added the awaiting responseIssues waiting for a reply from the OP or another party labelApr 9, 2025
kirkwaiblingerand others added3 commitsApril 9, 2025 19:18
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
bradzacher
bradzacher previously approved these changesApr 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.

kirkwaiblinger and JoshuaKGoldberg reacted with thumbs up emoji
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.

kirkwaiblinger and JoshuaKGoldberg reacted with thumbs up emoji
@github-actionsgithub-actionsbot removed 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge awaiting responseIssues waiting for a reply from the OP or another party labelsApr 10, 2025
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg 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

@JoshuaKGoldbergJoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelApr 10, 2025
Copy link
Member

@bradzacherbradzacher 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

kirkwaiblinger and Haprog reacted with laugh emoji
@bradzacherbradzacher merged commit69e2f6c intotypescript-eslint:mainApr 10, 2025
60 of 62 checks passed
@kirkwaiblingerkirkwaiblinger deleted the add-flat-configs-to-plugin branchApril 11, 2025 03:51
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsApr 19, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@armano2armano2armano2 left review comments

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

@bradzacherbradzacherbradzacher approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhancement: support ESLint's newdefineConfig string config references
4 participants
@kirkwaiblinger@armano2@JoshuaKGoldberg@bradzacher

[8]ページ先頭

©2009-2025 Movatter.jp