Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(typescript-estree): infers singleRun as true for project service#11327

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

Merged
bradzacher merged 23 commits intotypescript-eslint:mainfromnayounsang:singlerun-true
Nov 23, 2025

Conversation

@nayounsang
Copy link
Contributor

PR Checklist

Overview

Asproject is null whenprojectService is true, we have to fix validate logic

Turtle-Hwan reacted with thumbs up emoji
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@nayounsang!

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 onhttps://opencollective.com/typescript-eslint.

@netlify
Copy link

netlifybot commentedJun 21, 2025
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commitb483190
🔍 Latest deploy loghttps://app.netlify.com/projects/typescript-eslint/deploys/6909f96ce4a3fa0008cc3099
😎 Deploy Previewhttps://deploy-preview-11327--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: 91 (🔴 down 5 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 yourNetlify project configuration.

@nx-cloud
Copy link

nx-cloudbot commentedJun 21, 2025
edited
Loading

View yourCI Pipeline Execution ↗ for commitb483190

CommandStatusDurationResult
nx test eslint-plugin --coverage=false✅ Succeeded5m 21sView ↗
nx run-many -t lint✅ Succeeded3m 13sView ↗
nx run-many -t typecheck✅ Succeeded1m 54sView ↗
nx test typescript-estree --coverage=false✅ Succeeded19sView ↗
nx test eslint-plugin-internal --coverage=false✅ Succeeded11sView ↗
nx run types:build✅ Succeeded5sView ↗
nx run integration-tests:test✅ Succeeded4sView ↗
nx run generate-configs✅ Succeeded8sView ↗
Additional runs (29)✅ Succeeded...View ↗

☁️Nx Cloud last updated this comment at2025-11-04 13:14:21 UTC

@nayounsangnayounsang marked this pull request as draftJune 21, 2025 11:14
@nayounsang
Copy link
ContributorAuthor

Why tests was failed inCI?
In my local, there was no problem.

@nayounsang
Copy link
ContributorAuthor

nayounsang commentedJun 23, 2025
edited
Loading

Can't reproduce test errors in local development.
In test,parseSettings.singleRun frompackages/typescript-estree/src/parser.ts/parseAndGenerateServices isfalse.
So, it doesn't seem to be related to this commit.
I verified that the test results and (of course) the singleRun results did not change before and after the commit.
This work only affectssingleRun.

@nayounsangnayounsang marked this pull request as ready for reviewJune 23, 2025 08:07
@JoshuaKGoldbergJoshuaKGoldberg added awaiting responseIssues waiting for a reply from the OP or another party and removed awaiting responseIssues waiting for a reply from the OP or another party labelsJul 14, 2025
@JamesHenryJamesHenryforce-pushed thesinglerun-true branch 2 times, most recently fromfcdcb41 tode26768CompareJuly 15, 2025 19:27
@JamesHenry
Copy link
Member

@JoshuaKGoldberg@nayounsang Ok this one drove me a little insane working my way to a reproduction, but it actually has nothing to do with caching, it is all to do with whetherCI=true is set or not (which is why it happens only on CI and not locally by default).

In other words, you should be able to repro with:

CI=true TYPESCRIPT_ESLINT_PROJECT_SERVICE=true nxtest typescript-estree --skip-nx-cache
kirkwaiblinger, nayounsang, and JoshuaKGoldberg reacted with thumbs up emoji

@nayounsang
Copy link
ContributorAuthor

Thanks for helping us find the cause of this weird bug. Now let's fix our logic to fix the CI test failure.

@nayounsangnayounsang marked this pull request as draftJuly 18, 2025 11:22
@codecov
Copy link

codecovbot commentedJul 18, 2025
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.66%. Comparing base (5ec8c58) to head (b483190).
⚠️ Report is 199 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@##             main   #11327      +/-   ##==========================================- Coverage   90.86%   90.66%   -0.20%==========================================  Files         503      518      +15       Lines       51028    52435    +1407       Branches     8418     8686     +268     ==========================================+ Hits        46366    47541    +1175- Misses       4648     4880     +232  Partials       14       14
FlagCoverage Δ
unittest90.66% <100.00%> (-0.20%)⬇️

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

Files with missing linesCoverage Δ
...escript-estree/src/parseSettings/inferSingleRun.ts100.00% <100.00%> (ø)

... and84 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.

@nayounsang
Copy link
ContributorAuthor

nayounsang commentedJul 19, 2025
edited
Loading

Hi@JoshuaKGoldberg . Thank you for always helping me. I would like to ask you a question.
I want to test that singleRun works without any problems even whenextraFileExtensions is set &projectService is truthy. But, I don't know how to write test scenarios and where to test them.

(I can see the processingextraFileExtensions logic inpackages/typescript-estree/src/useProgramFromProjectService.ts
, but... I don't know if it's related to this issue.)

@nayounsang
Copy link
ContributorAuthor

nayounsang commentedJul 21, 2025
edited
Loading

WhensingleRun &projectService is true and givenextraFileExtensions inpackages/typescript-estree/tests/lib/semanticInfo-singleRun.test.ts, check the program is generated.

constresultProgram=parseAndGenerateServices(code,options).services.program;expect(resultProgram).toStrictEqual(mockProgram);

Because, the code is to solve the bug that occurs when programInstance is not found in the singleRun environment.

@nayounsangnayounsang marked this pull request as ready for reviewJuly 21, 2025 08:45
it('should create program when extraFileExtensions is provided in projectService and singleRun mode',()=>{
vi.stubEnv('TSESTREE_SINGLE_RUN','true');

constresultProgram=parseAndGenerateServices(code,{
Copy link
ContributorAuthor

@nayounsangnayounsangJul 21, 2025
edited
Loading

Choose a reason for hiding this comment

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

As asked, I'm not sure if testing like this is desirable.
parseAndGenerateService -> getProgramAndAST (in singleRun, get AST and program) -> useProgramFromProjectService -> extraFileExtensions

Choose a reason for hiding this comment

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

Yeah I'd remove this test change.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I committed

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

The first&& added seems off to me, but the other looks good. Am I reading it right?

it('should create program when extraFileExtensions is provided in projectService and singleRun mode',()=>{
vi.stubEnv('TSESTREE_SINGLE_RUN','true');

constresultProgram=parseAndGenerateServices(code,{

Choose a reason for hiding this comment

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

Yeah I'd remove this test change.

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelSep 3, 2025
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelSep 14, 2025
@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelSep 15, 2025
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelOct 27, 2025
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Ok I think we're pretty much there - just the one logical change. 🚀

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelNov 3, 2025
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelNov 4, 2025
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment
edited
Loading

Choose a reason for hiding this comment

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

✨ great, thanks!

Since I was so involved with this, someone else from @typescript-eslint/triage-team should probably take a look too.

@JoshuaKGoldbergJoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelNov 4, 2025
@bradzacherbradzacher merged commit75cc7d2 intotypescript-eslint:mainNov 23, 2025
67 of 68 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JamesHenryJamesHenryJamesHenry left review comments

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

@bradzacherbradzacherbradzacher approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug(typescript-estree): never infers singleRun as true for project service

4 participants

@nayounsang@JamesHenry@JoshuaKGoldberg@bradzacher

[8]ページ先頭

©2009-2025 Movatter.jp