8000 fix: typescript peer dependency by rtritto · Pull Request #10373 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

fix: typescript peer dependency #10373

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2024
Merged

fix: typescript peer dependency #10373

merged 1 commit into from
Dec 4, 2024

Conversation

rtritto
Copy link
Contributor
@rtritto rtritto commented Nov 23, 2024 •
edited
Loading

PR Checklist

Overview

typescript is a mandatory peer dependency (peerDepedency of ts-api-utils) and not optional peer dependency (it needs to be removed from peerDepedencyMeta).

Info

  • ts-api-utils has typescript as peer dependency:

    // ts-api-utils/package.json
{
  "peerDependencies": {
    "typescript": ">=4.2.0"
  }
}

Changes

  • @typescript-eslint/typescript-estree uses ts-api-utils dependency that has typescript as peer dependency (see Info)

  • @typescript-eslint/utils uses @typescript-eslint/typescript-estree dependency that uses ts-api-utils dependency. It needs typescript peer dependency

  • @typescript-eslint/parser uses @typescript-eslint/typescript-estree dependency that uses ts-api-utils dependency. It needs typescript peer dependency

  • @typescript-eslint/type-utils uses @typescript-eslint/typescript-estree and @typescript-eslint/utils dependencies that use ts-api-utils dependency. It needs typescript peer dependency

  • @typescript-eslint/eslint-plugin uses @typescript-eslint/utils dependency that uses @typescript-eslint/typescript-estree dependency that has typescript as peer dependency

  • typescript-eslint uses @typescript-eslint/parser and @typescript-eslint/utils dependencies that uses @typescript-eslint/typescript-estree dependency that has typescript as peer dependency

Issue Reproduction

Case where typescript is omitted by eslint-plugin-solid because @typescript-eslint/utils din't has typescript as mandatory peer dependency.

  • yarn init -y
  • yarn set version berry
  • yarn add -D eslint typescript eslint-plugin-solid
  • (optional) yarn explain peer-requirements and see logs 
    pcc27b → ✘ eslint-plugin-solid@npm:0.14.4 [3223e] doesn't provide typescript to @typescript-eslint/utils@npm:8.15.0 [e3d25] and 2 other dependencies
  • (detailed) yarn explain peer-requirements pcc27b and see logs 
    Package eslint-plugin-solid@npm:0.14.4 [3223e] is requested to provide typescript by its descendants

eslint-plugin-solid@npm:0.14.4 [3223e]
└─ @typescript-eslint/utils@npm:8.15.0 [e3d25] (via *)
   └─ @typescript-eslint/typescript-estree@npm:8.15.0 [0f560] (via *)
      └─ ts-api-utils@npm:1.3.0 [37728] (via >=4.2.0)

✘ Package eslint-plugin-solid@npm:0.14.4 [3223e] does not provide typescript.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @rtritto!

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.

@netlify Netlify
Copy link
netlify bot commented Nov 23, 2024 •
edited
Loading

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit abc7931
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/674df03ecc15b7000840288d
😎 Deploy Preview https://deploy-preview-10373--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 6 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (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.

@nx-cloud Nx Cloud
Copy link
nx-cloud bot commented Nov 23, 2024 •
edited
Loading

☁️ Nx Cloud Report

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

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 32 targets

Sent with 💌 from NxCloud.

@rtritto
Copy link
Contributor Author
rtritto commented Nov 23, 2024

Related #9744

FYI @ethanwu10 @JoshuaKGoldberg @jnoordsij @bradzacher

bradzacher
bradzacher requested changes Nov 23, 2024
View reviewed changes
@rtritto rtritto force-pushed the fix-typescript-peer-deps branch 2 times, most recently from d22d19b to e08bcad Compare November 23, 2024 20:59
@rtritto
Copy link
Contributor Author
rtritto commented Nov 24, 2024

Added reproduction

@rtritto rtritto force-pushed the fix-typescript-peer-deps branch from e08bcad to 0ae44b8 Compare November 25, 2024 20:52
@codecov Codecov
Copy link
codecov bot commented Nov 25, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.76%. Comparing base (2c8a75e) to head (abc7931).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #10373   +/-   ##
=======================================
  Coverage   86.76%   86.76%           
=======================================
  Files         444      444           
  Lines       15311    15311           
  Branches     4457     4457           
=======================================
  Hits        13285    13285           
  Misses       1672     1672           
  Partials      354      354
Flag Coverage Δ
unittest 86.76% <ø> (ø)

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

@rtritto rtritto force-pushed the fix-typescript-peer-deps branch from 0ae44b8 to abc7931 Compare December 2, 2024 17:37
@rtritto
Copy link
Contributor Author
rtritto commented Dec 2, 2024

Rebased

bradzacher
bradzacher approved these changes Dec 4, 2024
View reviewed changes
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.

thanks for doing this and sticking with us!

@bradzacher bradzacher merged commit 4cb2cf8 into typescript-eslint:main Dec 4, 2024
62 checks passed
@rtritto rtritto deleted the fix-typescript-peer-deps branch December 4, 2024 17:06
@AviVahl
Copy link
AviVahl commented Dec 9, 2024

@bradzacher could you guys please make the semver range more lenient?
we'd like to be able to upgrade to 5.8 even before the official typescript-eslint support (seeing the long warning about it)

@ljharb ljharb mentioned this pull request Dec 9, 2024
Closed
4 tasks
@ljharb
Copy link
ljharb commented Dec 9, 2024 •
edited
Loading

This PR was a breaking change (adding a peer dependency always is); see #10480.

The proper fix here was probably downgrading to a version of ts-api-utils that doesn't have the peer dep.

renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Dec 9, 2024
@renovate
fix(deps): update typescript-eslint monorepo to v8.18.0
46e638c 
##### [v8.18.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8180-2024-12-09)

##### 🚀 Features

-   **eslint-plugin:** \[switch-exhaustiveness-check] add support for "no default" comment ([#10218](typescript-eslint/typescript-eslint#10218))
-   **eslint-plugin:** \[no-deprecated] report on super call of deprecated constructor ([#10397](typescript-eslint/typescript-eslint#10397))

##### 🩹 Fixes

-   **eslint-plugin:** \[use-unknown-in-catch-callback-variable] only flag function literals ([#10436](typescript-eslint/typescript-eslint#10436))
-   **eslint-plugin:** \[no-base-to-string] handle more robustly when multiple `toString()` declarations are present for a type ([#10432](typescript-eslint/typescript-eslint#10432))
-   **eslint-plugin:** \[no-deprecated] check if a JSX attribute is deprecated ([#10374](typescript-eslint/typescript-eslint#10374))
-   typescript peer dependency ([#10373](typescript-eslint/typescript-eslint#10373))

##### ❤️  Thank You

-   Kim Sang Du [@developer-bandi](https://github.com/developer-bandi)
-   Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger)
-   mdm317
-   rtritto

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.
@bradzacher
Copy link
Member

@AviVahl the range represents what we officially support. As we have seen many times in the past the newer versions of TS could cause crashes until we release support. If you want to use a newer version of TS then you can use your package manager's flag to specify you want to ignore the peer dependency ranges.

@bradzacher
Copy link
Member

Regardless - as per our contributing guide - we do not seek comments on PRs because it's not searchable from the issue search. Please file an issue if you wish to discuss further.

@typescript-eslint typescript-eslint locked and limited conversation to collaborators Dec 9, 2024
@JoshuaKGoldberg
Copy link
Member

This PR was a breaking change (adding a peer dependency always is); see #10480.

Just noting for visibility from #10480: the PR was not a breaking change. It was a bugfix in that we weren't properly declaring the actual peer dependency range before.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bradzacher bradzacher bradzacher approved these changes

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

Successfully merging this pull request may close these issues.

Bug: Missing peer dependencies
5 participants
@rtritto @AviVahl @ljharb @bradzacher @JoshuaKGoldberg
0