typescript-eslint/typescript-eslintPublic

NotificationsYou must be signed in to change notification settings
Fork2.8k
Star16k

feat: update to lib types for TS 5.2 RC#7451

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

Jump to bottom

Merged

bradzacher merged 8 commits intotypescript-eslint:mainfromsosukesuzuki:update-ts-5.2

Aug 13, 2023

Merged

feat: update to lib types for TS 5.2 RC#7451

bradzacher merged 8 commits intotypescript-eslint:mainfromsosukesuzuki:update-ts-5.2

Aug 13, 2023

Conversation

Copy link

Contributor

sosukesuzuki commentedAug 10, 2023•
edited
Loading

PR Checklist

Addresses an existing open issue: RC support ofTypeScript 5.2 Support #7155
That issue was marked asaccepting prs
Steps inContributing were taken

Overview

Update TypeScript to 5.2.1-rc

Copy link

Contributor

typescript-eslintbot commentedAug 10, 2023

Thanks for the PR,@sosukesuzuki!

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

netlifybot commentedAug 10, 2023•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`2db6ce1`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/64d8810036f2830008c6ddda
😎 Deploy Preview	https://deploy-preview-7451--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

Copy link

codecovbot commentedAug 10, 2023•
edited
Loading

Codecov Report

Merging#7451 (2db6ce1) intomain (c225b12) willdecrease coverage by2.10%.
The diff coverage is100.00%.

Additional details and impacted files

@@            Coverage Diff             @@##             main    #7451      +/-   ##==========================================- Coverage   87.34%   85.25%   -2.10%==========================================  Files         381      386       +5       Lines       13336    13358      +22       Branches     3943     3943              ==========================================- Hits        11648    11388     -260- Misses       1308     1593     +285+ Partials      380      377       -3

Flag	Coverage Δ
unittest	`85.25% <100.00%> (-2.10%)`	⬇️

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

Files Changed	Coverage Δ
packages/scope-manager/src/lib/decorators.ts	`100.00% <ø> (ø)`
packages/scope-manager/src/lib/dom.iterable.ts	`100.00% <ø> (ø)`
packages/scope-manager/src/lib/dom.ts	`100.00% <ø> (ø)`
packages/scope-manager/src/lib/es5.ts	`100.00% <ø> (ø)`
...ckages/scope-manager/src/lib/webworker.iterable.ts	`100.00% <ø> (ø)`
packages/scope-manager/src/lib/webworker.ts	`100.00% <ø> (ø)`
packages/typescript-estree/src/version-check.ts	`100.00% <ø> (ø)`
packages/scope-manager/src/lib/es2017.date.ts	`100.00% <100.00%> (ø)`
packages/scope-manager/src/lib/es2017.ts	`100.00% <100.00%> (ø)`
...ackages/scope-manager/src/lib/es2023.collection.ts	`100.00% <100.00%> (ø)`
... and7 more

... and7 files with indirect coverage changes

Sosuke Suzuki added6 commits

August 11, 2023 01:29

feat: update totypescript@5.2.1-rc`

63dc18b

fix: add patch for 5.2.1-rc

53ae84c

feat: rungenerate-lib

1517f0b

feat: updateSUPPORTED_TYPESCRIPT_VERSIONS

afb048d

feat: updateversion-check.ts

715b0e6

fix: updatetsconfig.spec.json forscope-manger

5cfbf54

sosukesuzuki force-pushed theupdate-ts-5.2 branch from4d0ba91 to5cfbf54Compare

August 10, 2023 16:29

Copy link

nx-cloudbot commentedAug 10, 2023•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit2db6ce1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 37 targets

Sent with 💌 fromNxCloud.

sosukesuzuki marked this pull request as ready for review

August 10, 2023 17:17

JoshuaKGoldberg reviewed

Aug 12, 2023

View reviewed changes

packages/typescript-estree/src/version-check.ts

		'4.9',
		'5.0',
		'5.1',
		'5.2',

Copy link

Member

JoshuaKGoldbergAug 12, 2023

Choose a reason for hiding this comment

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

...huh. Odd that 5.1 wasn't here before.

JoshuaKGoldberg reviewed

Aug 12, 2023

View reviewed changes

packages/scope-manager/tsconfig.spec.json Outdated

		"types": ["jest","node"]
		"module":"Node16",
		"types": ["jest","node"],
		"moduleResolution":"Node16"

Copy link

Member

JoshuaKGoldbergAug 12, 2023•
edited
Loading

Choose a reason for hiding this comment

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

Why this change? Tests still work for me locally after reverting it.

Copy link

Member

bradzacherAug 13, 2023

Choose a reason for hiding this comment

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

But does it typecheck fine without it?
There's a new error in this version to ensure that things are configured correctly.

Copy link

Member

JoshuaKGoldbergAug 13, 2023

Choose a reason for hiding this comment

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

Yeah I like moving to the newer module & module resolution settings, just curious.

Also, perhttps://www.typescriptlang.org/tsconfig#moduleResolution,moduleResolution matchesmodule by default withnode16 and other newer targets.

Copy link

ContributorAuthor

sosukesuzukiAug 13, 2023

Choose a reason for hiding this comment

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

Without this change, a type error will occur.

Copy link

Member

JoshuaKGoldbergAug 13, 2023

Choose a reason for hiding this comment

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

Ah!https://devblogs.microsoft.com/typescript/announcing-typescript-5-2-rc/#module-and-moduleresolution-must-match-under-recent-node-js-settings

       tsconfig.spec.json:5:15 - error TS5110: Option 'module' must be set to 'Node16' when option 'moduleResolution' is set to 'Node16'.              5     "module": "CommonJS",                       ~~~~~~~~~~

Fun! In that case, I think we can get away with just the"module": "Node16":

Suggested change

"moduleResolution": "Node16"