Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript-eslint): declare peer dependency on utils to ensure npm correctly installs dependencies #8738

Merged
merged 3 commits into from Mar 21, 2024

Conversation

bradzacher
Copy link
Member

PR Checklist

Overview

typescript-eslint is currently "naughty" and accesses types from /utils via a transitive dependency.
This means that the types we receive depend on how the package manager installs things.

pnpm and npm both do a bad job of installing dependencies in the most sane way (npm/cli#7300) and given the current ecosystem it's possible to get an old major for /utils installed at the root instead of v7. This means TS will get the incorrect types for /utils and then the config types break.

This adds a hard dep on /utils to force the relationship so that all package managers do the same thing and either hoist the /utils dependency to the root, or at least ensure that /utils is nested within typescript-eslint.

I considered making this a peer dep - but considering /eslint-plugin already depends on /utils - it's always installed (i.e. we're not adding an extra dep) - so no need to do anything complicated that might be misinterpreted by package managers.

@bradzacher bradzacher added the bug Something isn't working label Mar 20, 2024
@typescript-eslint
Copy link
Contributor

Thanks for the PR, @bradzacher!

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

netlify bot commented Mar 20, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 87ae83c
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/65fb5bc6ba959300089759fe
😎 Deploy Preview https://deploy-preview-8738--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 (no change 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.

@typescript-eslint typescript-eslint deleted a comment Mar 21, 2024
@bradzacher bradzacher merged commit 59bbf41 into main Mar 21, 2024
60 checks passed
@bradzacher bradzacher deleted the 8732-typescript-eslint-dep branch March 21, 2024 07:20
peanutenthusiast pushed a commit to peanutenthusiast/typescript-eslint that referenced this pull request Mar 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Type error when using ConfigWithExtends
2 participants