Skip to content

Conversation

Josh-Cena
Copy link
Member

PR Checklist

Overview

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @Josh-Cena!

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 Jun 3, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 546a854
🔍 Latest deploy log https://app.netlify.com/projects/typescript-eslint/deploys/68e3b5eef92f0f00084e17c1
😎 Deploy Preview https://deploy-preview-9232--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: 90 (🟢 up 12 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (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 project configuration.

Copy link

nx-cloud bot commented Jun 3, 2024

View your CI Pipeline Execution ↗ for commit 546a854

Command Status Duration Result
nx test eslint-plugin --coverage=false ✅ Succeeded 5m 2s View ↗
nx run-many -t lint ✅ Succeeded 3m 10s View ↗
nx run-many -t typecheck ✅ Succeeded 2m 8s View ↗
nx run types:build ✅ Succeeded 5s View ↗
nx test eslint-plugin-internal --coverage=false ✅ Succeeded 9s View ↗
nx run integration-tests:test ✅ Succeeded 4s View ↗
nx test typescript-estree --coverage=false ✅ Succeeded 20s View ↗
nx run generate-configs ✅ Succeeded 9s View ↗
Additional runs (29) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-10-06 12:42:14 UTC

@Josh-Cena Josh-Cena marked this pull request as draft June 3, 2024 17:14
@Josh-Cena
Copy link
Member Author

Converting it to draft, because TS is very wack about private identifiers: console.log(#a) is also valid.

@JoshuaKGoldberg
Copy link
Member

👋 just checking in @Josh-Cena, is there anything we should do with this?

@JoshuaKGoldberg
Copy link
Member

Chatted 1:1, this is blocked on a resolution to microsoft/TypeScript#58754.

@JoshuaKGoldberg JoshuaKGoldberg added the blocked by another issue Issues which are not ready because another issue needs to be resolved first label Nov 10, 2024
@kirkwaiblinger
Copy link
Member

Since the TS fix has been languishing with no progress for over a year, can we just move forward with this on our side? Or do we need to keep this hard-blocked?

@JoshuaKGoldberg
Copy link
Member

Yeah, I think we should consider ourselves un-blocked at this point.

@JoshuaKGoldberg JoshuaKGoldberg removed the blocked by another issue Issues which are not ready because another issue needs to be resolved first label Sep 29, 2025
Copy link

codecov bot commented Sep 29, 2025

Codecov Report

❌ Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.72%. Comparing base (9530e02) to head (546a854).
⚠️ Report is 39 commits behind head on main.

Files with missing lines Patch % Lines
packages/typescript-estree/src/convert.ts 23.07% 10 Missing ⚠️

❌ Your patch status has failed because the patch coverage (23.07%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9232      +/-   ##
==========================================
- Coverage   90.93%   90.72%   -0.21%     
==========================================
  Files         506      516      +10     
  Lines       51394    52027     +633     
  Branches     8482     8596     +114     
==========================================
+ Hits        46735    47203     +468     
- Misses       4646     4810     +164     
- Partials       13       14       +1     
Flag Coverage Δ
unittest 90.72% <23.07%> (-0.21%) ⬇️

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

Files with missing lines Coverage Δ
...s/ast-spec/src/expression/BinaryExpression/spec.ts 100.00% <ø> (ø)
packages/typescript-estree/src/convert.ts 31.12% <23.07%> (+0.89%) ⬆️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM! I think I'm mildly in favor of @kirkwaiblinger's suggestion, so I'll defer to you two on that. 🙂

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review September 29, 2025 16:28
@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Oct 6, 2025
@JoshuaKGoldberg JoshuaKGoldberg merged commit d29bd0a into typescript-eslint:main Oct 6, 2025
65 of 66 checks passed
@Josh-Cena Josh-Cena deleted the private-binary-expr-ast-invariant branch October 6, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AST tightening: PrivateIdentifier can only be LHS of BinaryExpression[operator='in']

3 participants