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 TypeScript 5.8#10903

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

Merged

Conversation

dirkluijk
Copy link
Contributor

@dirkluijkdirkluijk commentedFeb 28, 2025
edited
Loading

Closes#10884

PR Checklist

Overview

lib.d.ts Updates

We will need to regenerate our types within scope-manager.

I am still figuring out if I need to do something for this.

Never mind, done.

silverwind, mkosir, jnesta-lh, stephenwade, rtritto, SergeAstapov, lin72h, ibrahimBeladi, MikeMcC399, siketyan, and 9 more reacted with thumbs up emojinwalters512, azu, mkosir, jnesta-lh, ivyhjk, stephenwade, rtritto, lin72h, ibrahimBeladi, paescuj, and 4 more reacted with heart emojicharliez0, mkosir, JoaoPauloCMarra, robinlahtinen, jnesta-lh, UziTech, rtritto, lin72h, aqeelat, ibrahimBeladi, and 5 more reacted with rocket emoji
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@dirkluijk!

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.

@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedFeb 28, 2025
edited
Loading

View yourCI Pipeline Execution ↗ for commit749d7d8.

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

☁️Nx Cloud last updated this comment at2025-03-02 18:48:10 UTC

@netlifyNetlify
Copy link

netlifybot commentedFeb 28, 2025
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit749d7d8
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/67c4a3de53feac0008d115af
😎 Deploy Previewhttps://deploy-preview-10903--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: 99 (🟢 up 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (🟢 up 8 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.

@dirkluijkdirkluijk changed the title[WIP] feat: support TypeScript 5.8feat: support TypeScript 5.8Feb 28, 2025
@dirkluijkdirkluijk mentioned this pull requestFeb 28, 2025
@dirkluijk
Copy link
ContributorAuthor

dirkluijk commentedFeb 28, 2025
edited
Loading

Hmm, there seems to be a linting issue intools/release/release.mts. Not sure why, will look into it.

Seems to be an issue withnx/release (TS2307: Cannot find module nx/release or its corresponding type declarations.).

Edit: fixed it, seems like TS 5.8 handles this import differently, maybe because it's a CommonJS package and we're using NodeNext. It's not ESLint related anyway.

jnesta-lh reacted with thumbs up emoji

@codecovCodecov
Copy link

codecovbot commentedFeb 28, 2025
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.44%. Comparing base(3c7b8ff) to head(749d7d8).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@##             main   #10903   +/-   ##=======================================  Coverage   87.43%   87.44%           =======================================  Files         468      469    +1       Lines       16040    16047    +7       Branches     4649     4649           =======================================+ Hits        14025    14032    +7  Misses       1658     1658             Partials      357      357
FlagCoverage Δ
unittest87.44% <100.00%> (+<0.01%)⬆️

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

Files with missing linesCoverage Δ
packages/scope-manager/src/lib/dom.iterable.ts100.00% <ø> (ø)
packages/scope-manager/src/lib/dom.ts100.00% <ø> (ø)
packages/scope-manager/src/lib/esnext.float16.ts100.00% <100.00%> (ø)
packages/scope-manager/src/lib/esnext.promise.ts100.00% <ø> (ø)
packages/scope-manager/src/lib/esnext.ts100.00% <100.00%> (ø)
packages/scope-manager/src/lib/index.ts100.00% <100.00%> (ø)
...ckages/scope-manager/src/lib/webworker.iterable.ts100.00% <ø> (ø)
packages/scope-manager/src/lib/webworker.ts100.00% <ø> (ø)
...ipt-estree/src/parseSettings/warnAboutTSVersion.ts94.44% <100.00%> (ø)

JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesMar 2, 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, very nicely done. Thanks for helping out! 🙌

Just a couple of small things. Nothing that we can't do before the weekly release tomorrow if you don't have time.

releaseChangelog,
releasePublish,
releaseVersion,
}from'nx/release/index.js';

Choose a reason for hiding this comment

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

Unrelated change, is there a reason to include it here?

This showed up in other places -at least#10887, cc @kirkwaiblinger- so maybe we need to do this inmain?

bradzacher reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yeah, see#10903 (comment)

My theory is - but I haven't checked it yet - that TypeScript 5.8 behaves slightly different with deep imports in CommonJS modules that don't have specific secondary entry points.

On TS 5.7 this is not an issue, so I think it belongs to this PR. Although we could also add this tomain first of course.

kirkwaiblinger reacted with thumbs up emoji

Choose a reason for hiding this comment

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

We had a poke around, and the change is due tomicrosoft/TypeScript#61057. TL;DR you can't omit the/index.js whenimporting from ESM in node. Seemingly executing with tsx rather than node has been preventing this from being a runtime error for us so far, but the TS 5.8 error is correct, and, accordingly, so is this change. 👍 Also@JamesHenry is aware and might tweak things on thenx side (whether justthe docs or adding a subpathexports field)

Choose a reason for hiding this comment

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

As for plan of action - both#10887 and this PR genuinely need this change for different reasons (TS type-checking error here, and runtime error when using node 23.6, which#10887 resolves), so I'd say let's just keep the change in both PRs and whichever is merged first is merged first 🤷‍♂️

dirkluijk and bradzacher reacted with thumbs up emoji
bradzacher
bradzacher previously approved these changesMar 2, 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.

Same comments as joshg

Copy link
Member

@bradzacherbradzacher left a comment
edited
Loading

Choose a reason for hiding this comment

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

lgtm15

merging without Josh's re-review as his comments were resolved appropriately. Wanna make sure this is in for the next release.

shrujalshah28 and dirkluijk reacted with hooray emoji
@bradzacherbradzacher merged commit9674629 intotypescript-eslint:mainMar 3, 2025
93 checks passed
@dirkluijkdirkluijk deleted the feat/ts-5.8-support branchMarch 3, 2025 07:55
renovatebot added a commit to andrei-picus-tink/auto-renovate that referenced this pull requestMar 5, 2025
| datasource | package                          | from   | to     || ---------- | -------------------------------- | ------ | ------ || npm        | @typescript-eslint/eslint-plugin | 8.24.0 | 8.26.0 || npm        | @typescript-eslint/parser        | 8.24.0 | 8.26.0 |## [v8.26.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8260-2025-03-03)##### 🚀 Features-   **eslint-plugin:** \[unified-signatures] support ignoring overload signatures with different JSDoc comments ([#10781](typescript-eslint/typescript-eslint#10781))-   **eslint-plugin:** \[explicit-module-boundary-types] add an option to ignore overload implementations ([#10889](typescript-eslint/typescript-eslint#10889))-   **eslint-plugin:** \[no-unused-var] handle implicit exports in declaration files ([#10714](typescript-eslint/typescript-eslint#10714))-   support TypeScript 5.8 ([#10903](typescript-eslint/typescript-eslint#10903))-   **eslint-plugin:** \[no-unnecessary-type-parameters] special case tuples and parameter location arrays as single-use ([#9536](typescript-eslint/typescript-eslint#9536))##### 🩹 Fixes-   **eslint-plugin:** \[no-unnecessary-type-assertion] handle unknown ([#10875](typescript-eslint/typescript-eslint#10875))-   **eslint-plugin:** \[no-invalid-void-type] report `accessor` properties with an invalid `void` type ([#10864](typescript-eslint/typescript-eslint#10864))-   **eslint-plugin:** \[unified-signatures] does not differentiate truly private methods ([#10806](typescript-eslint/typescript-eslint#10806))##### ❤️ Thank You-   Andrea Simone Costa [@jfet97](https://github.com/jfet97)-   Dirk Luijk [@dirkluijk](https://github.com/dirkluijk)-   Ronen Amiel-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)-   Yukihiro Hasegawa [@y-hsgw](https://github.com/y-hsgw)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.## [v8.25.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8250-2025-02-24)##### 🚀 Features-   **eslint-plugin:** \[no-misused-spread] add suggestions ([#10719](typescript-eslint/typescript-eslint#10719))##### 🩹 Fixes-   **eslint-plugin:** \[prefer-nullish-coalescing] report on chain expressions in a ternary ([#10708](typescript-eslint/typescript-eslint#10708))-   **eslint-plugin:** \[no-deprecated] report usage of deprecated private identifiers ([#10844](typescript-eslint/typescript-eslint#10844))-   **eslint-plugin:** \[unified-signatures] handle getter-setter ([#10818](typescript-eslint/typescript-eslint#10818))##### ❤️ Thank You-   Olivier Zalmanski [@OlivierZal](https://github.com/OlivierZal)-   Ronen Amiel-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.## [v8.24.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8241-2025-02-17)##### 🩹 Fixes-   **eslint-plugin:** \[class-methods-use-this] check `accessor` methods with a function initializer ([#10796](typescript-eslint/typescript-eslint#10796))-   **eslint-plugin:** \[no-misused-promises] don't report on `static` `accessor` properties ([#10814](typescript-eslint/typescript-eslint#10814))-   **eslint-plugin:** \[no-deprecated] don't report on deprecated `accessor` property declaration ([#10813](typescript-eslint/typescript-eslint#10813))-   **eslint-plugin:** \[explicit-member-accessibility] check `accessor` class properties for missing accessibility modifier ([#10805](typescript-eslint/typescript-eslint#10805))-   **eslint-plugin:** \[explicit-module-boundary-types] check `accessor` class properties with a function initializer ([#10804](typescript-eslint/typescript-eslint#10804))-   **eslint-plugin:** \[prefer-return-this-type] check `accessor` properties with a function initializer ([#10794](typescript-eslint/typescript-eslint#10794))-   **eslint-plugin:** \[consistent-generic-constructors] check `accessor` class properties ([#10789](typescript-eslint/typescript-eslint#10789))-   **eslint-plugin:** \[no-unsafe-assignment] report on an `any` value assigned as an initializer of an `accessor` property ([#10785](typescript-eslint/typescript-eslint#10785))-   **eslint-plugin:** \[no-unnecessary-template-expression] ignore enum and enum members ([#10782](typescript-eslint/typescript-eslint#10782))-   **eslint-plugin:** \[no-inferrable-types] handle accessor ([#10780](typescript-eslint/typescript-eslint#10780))##### ❤️ Thank You-   Ronen Amiel-   YeonJuanYou can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsMar 11, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@kirkwaiblingerkirkwaiblingerkirkwaiblinger left review comments

@bradzacherbradzacherbradzacher approved these changes

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TypeScript 5.8 Support
4 participants
@dirkluijk@JoshuaKGoldberg@bradzacher@kirkwaiblinger

[8]ページ先頭

©2009-2025 Movatter.jp