fix: add missing peer dependencies #9744

ethanwu10 · 2024-08-07T02:01:07Z

PR Checklist

Addresses an existing open issue: fixes Bug: Missing peer dependencies #9224
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Ensure all transitive peer dependencies are properly listed. This fixes peer-requirements warnings thrown by yarn.

typescript-eslint · 2024-08-07T02:01:09Z

Thanks for the PR, @ethanwu10!

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 · 2024-08-07T02:01:32Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`dd06088`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/66b2d565dee66b0007510772
😎 Deploy Preview	https://deploy-preview-9744--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 95 (no change from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (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 · 2024-08-07T02:01:58Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit dd06088. 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 2 targets

Sent with 💌 from NxCloud.

rtritto · 2024-08-27T08:33:11Z

Any update?

FYI @JoshuaKGoldberg

bradzacher · 2024-08-27T10:46:31Z

@rtritto The issue has not been marked as accepting PRs.
As a team we have yet to discuss it.

Please do not ping on issues or tag maintainers in comments.
We as VOLUNTEER maintainers will get to this when we have the bandwidth.

rtritto · 2024-09-19T18:52:42Z

I have this warnings after yarn explain peer-requirements:

p4ee77 → ✓ @typescript-eslint/parser@npm:8.6.0 [3223e] doesn't provide @types/supports-color to debug@npm:4.3.6 [1ff4b]
p42ec1 → ✓ @typescript-eslint/parser@npm:8.6.0 [3223e] doesn't provide supports-color to debug@npm:4.3.6 [1ff4b]
p5cc2b → ✓ @typescript-eslint/type-utils@npm:8.6.0 [8a609] doesn't provide @types/eslint to @typescript-eslint/utils@npm:8.6.0 [7f592] and 1 other dependency       
p0f521 → ✓ @typescript-eslint/type-utils@npm:8.6.0 [8a609] doesn't provide @types/supports-color to debug@npm:4.3.6 [1ff4b]
p6eab7 → ✘ @typescript-eslint/type-utils@npm:8.6.0 [8a609] doesn't provide eslint to @typescript-eslint/utils@npm:8.6.0 [7f592] and 1 other dependency
p31167 → ✓ @typescript-eslint/type-utils@npm:8.6.0 [8a609] doesn't provide supports-color to debug@npm:4.3.6 [1ff4b]
pa4871 → ✓ @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] doesn't provide @types/supports-color to debug@npm:4.3.6 [1ff4b]
p66f84 → ✓ @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] doesn't provide supports-color to debug@npm:4.3.6 [1ff4b]
p67afe → ✓ @typescript-eslint/typescript-estree@npm:8.6.0 [7f592] doesn't provide @types/supports-color to debug@npm:4.3.6 [1ff4b]
p1b301 → ✓ @typescript-eslint/typescript-estree@npm:8.6.0 [7f592] doesn't provide supports-color to debug@npm:4.3.6 [1ff4b]
pf4e07 → ✓ @typescript-eslint/utils@npm:8.6.0 [7f592] doesn't provide @types/typescript to @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] and 1 other dependency
pb85fd → ✘ @typescript-eslint/utils@npm:8.6.0 [7f592] doesn't provide typescript to @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] and 1 other dependency   
pa9c59 → ✓ @typescript-eslint/utils@npm:8.6.0 [8a609] doesn't provide @types/typescript to @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] and 1 other dependency
pafe94 → ✘ @typescript-eslint/utils@npm:8.6.0 [8a609] doesn't provide typescript to @typescript-eslint/typescript-estree@npm:8.6.0 [346c1] and 1 other dependency

I found a workaround (with yarn berry) to this issue:

# .yarnrc.yml
packageExtensions:
  "@typescript-eslint/type-utils@*":
    dependencies:
      eslint: ^8  # don't use v9
  "@typescript-eslint/utils@*":
    dependencies:
      typescript: "*"

There is an article written by author of yarn package manager: Implicit Transitive Peer Dependencies

rtritto · 2024-10-18T18:17:30Z

Also typescript-eslint if affected by this issue.

Updated workaround using only typescript-eslint:

# .yarnrc.yml
packageExtensions:
  "@typescript-eslint/type-utils@*":
    dependencies:
      eslint: "*"
  "@typescript-eslint/utils@*":
    dependencies:
      typescript: "*"
  typescript-eslint@*:
    peerDependencies:
      eslint: "*"

JoshuaKGoldberg

Thanks for sending this in, and sorry for the wait!

This seems reasonable & straightforward to me. Just one question on the utils package's meta.

JoshuaKGoldberg · 2024-11-10T16:46:41Z

packages/utils/package.json

@@ -78,6 +78,11 @@
    "rimraf": "*",
    "typescript": "*"
  },
+  "peerDependenciesMeta": {
+    "typescript": {


[Question] Shouldn't this also be listed in peerDependencies? Even if listing it in peerDependenciesMeta implicitly adds it there, it feels odd to me to make that implicit.

I just copied this from type-utils (which is where the missing dep for typescript comes from), which also does not list it as an explicit peerdep

typescript-eslint/packages/type-utils/package.json

Lines 64 to 68 in 42d275c

"peerDependenciesMeta": {

"typescript": {

"optional": true

}

},

Ha, nice, so this might be an existing issue in that package then? My vote is to fix in both packages.

Just came here to say that appearently at least both Yarn and pnpm have opted in the past to have "peerDependenciesMeta without peerDependencies" imply a peer dependency on *. This was done due to some (legacy?) npm behavior regarding peer dependencies, making package authors hesistant to list them. See pnpm/pnpm#2128.

But it's probably a better practice to explicitly list them; and given the age of the referenced issue probably safe enough as well.

The reason we used peerDependenciesMeta without peerDependencies was because there were versions of npm that did not support peerDependenciesMeta. This meant that when we used an explicit peerDependencies then users would be spammed with warnings saying they didn't have TS installed. This was a problem as some big packages included our plugins by default but only used them if the user opted in to TS which caused non-TS users to get spammed with missing peer dep errors.

A this point we could probably also include the explicit peerDependencies on TS but also there's no harm in not including it and having it inferred to ensure that nobody gets spammed with logs.

Do you know if npm also shares this behavior of peerDependenciesMeta implying peerDependencies? From a cursory search of the npm sources it seems to me like it doesn't: https://github.com/npm/cli/blob/75a3f1228865f426d8790be27f1258e501f2c450/workspaces/arborist/lib/node.js#L859-L871

Anyways, what do we think of merging this PR first and moving the discussion about whether or not to list an explicit peerdep to a different issue? We don't need to block fixing users' warnings on figuring out the proper manifest declarations 😅

I'm 👍 on that. If nobody else weighs in with a contrary opinion by our weekly release midday Monday EST then we can just merge as-is.

Now that you mention it, I do like the idea of having one release just adding peerDependenciesMeta, then a subsequent release adding peerDependencies on top. Just in case there's some bizarre edge case bug with the latter.

JoshuaKGoldberg

🙌 Thanks for your patience and working with us!

bradzacher

thanks for this!

fix: add missing peer dependencies

dd06088

Ensure all transitive peer dependencies are properly listed. This fixes peer-requirements warnings thrown by yarn.

JoshuaKGoldberg marked this pull request as draft August 27, 2024 14:09

bradzacher mentioned this pull request Sep 30, 2024

fix(typescript-eslint): add eslint as peer dependency #10079

Closed

3 tasks

JoshuaKGoldberg requested changes Nov 10, 2024

View reviewed changes

JoshuaKGoldberg marked this pull request as ready for review November 10, 2024 16:48

JoshuaKGoldberg approved these changes Nov 15, 2024

View reviewed changes

bradzacher approved these changes Nov 16, 2024

View reviewed changes

bradzacher merged commit 00a3a84 into typescript-eslint:main Nov 16, 2024
69 of 70 checks passed

scagood mentioned this pull request Nov 20, 2024

Missing peer dependency typescript eslint-stylistic/eslint-stylistic#614

Open

5 tasks

rtritto mentioned this pull request Nov 23, 2024

fix: typescript peer dependency #10373

Merged

3 tasks

github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add missing peer dependencies #9744

fix: add missing peer dependencies #9744

ethanwu10 commented Aug 7, 2024 •

edited by JoshuaKGoldberg

Loading

typescript-eslint bot commented Aug 7, 2024

netlify bot commented Aug 7, 2024 •

edited

Loading

nx-cloud bot commented Aug 7, 2024 •

edited

Loading

rtritto commented Aug 27, 2024

bradzacher commented Aug 27, 2024 •

edited

Loading

rtritto commented Sep 19, 2024

rtritto commented Oct 18, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg Nov 10, 2024

ethanwu10 Nov 10, 2024 •

edited

Loading

JoshuaKGoldberg Nov 10, 2024

jnoordsij Nov 11, 2024

bradzacher Nov 12, 2024 •

edited

Loading

ethanwu10 Nov 15, 2024

JoshuaKGoldberg Nov 15, 2024

JoshuaKGoldberg left a comment •

edited

Loading

bradzacher left a comment

fix: add missing peer dependencies #9744

fix: add missing peer dependencies #9744

Conversation

ethanwu10 commented Aug 7, 2024 • edited by JoshuaKGoldberg Loading

PR Checklist

Overview

typescript-eslint bot commented Aug 7, 2024

netlify bot commented Aug 7, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Aug 7, 2024 • edited Loading

☁️ Nx Cloud Report

rtritto commented Aug 27, 2024

bradzacher commented Aug 27, 2024 • edited Loading

rtritto commented Sep 19, 2024

rtritto commented Oct 18, 2024 • edited Loading

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Nov 10, 2024

Choose a reason for hiding this comment

ethanwu10 Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg Nov 10, 2024

Choose a reason for hiding this comment

jnoordsij Nov 11, 2024

Choose a reason for hiding this comment

bradzacher Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

ethanwu10 Nov 15, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Nov 15, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

bradzacher left a comment

Choose a reason for hiding this comment

ethanwu10 commented Aug 7, 2024 •

edited by JoshuaKGoldberg

Loading

netlify bot commented Aug 7, 2024 •

edited

Loading

nx-cloud bot commented Aug 7, 2024 •

edited

Loading

bradzacher commented Aug 27, 2024 •

edited

Loading

rtritto commented Oct 18, 2024 •

edited

Loading

ethanwu10 Nov 10, 2024 •

edited

Loading

bradzacher Nov 12, 2024 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading