fix(typescript-estree): adds support for project services using extended config files #9306

higherorderfunctor · 2024-06-08T02:05:23Z

PR Checklist

Addresses an existing open issue: fixes Bug(typescript-estree): Project services default project not working with extended tsconfigs #9205
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Changes packages/typescript-estree/src/create-program/createProjectService.ts to use tsserver.getParsedCommandLineOfConfigFile instead of tsserver.readConfigFile to process extended tsconfigs.
Moves the tsserver.getParsedCommandLineOfConfigFile boilerplate from packages/typescript-estree/src/create-program/useProvidedPrograms.ts into a new file packages/typescript-estree/src/create-program/getParsedConfigFile.ts to keep the code DRY.
Adds tests for single and multiple extends to packages/typescript-estree/tests/lib/createProjectService.test.ts. Also adds a couple tests to round out coverage for uncommon code paths to bring getParsedConfigFile.ts to 100% (tsserver.sys === undefined and triggering getCanonicalFileName with tsserver.formatDiagnostics).
Adds a new fixture at packages/typescript-estree/tests/fixtures/projectServicesComplex/.

A few implementation notes for the reviewer:

There is a hard any cast in packages/typescript-estree/src/create-program/createProjectService.ts. I left a code comment with reasoning for doing so. The original tsserver.readConfigFile was also any with a hard cast.
formatDiagnostics in the new packages/typescript-estree/src/create-program/getParsedConfigFile.ts file was placed inside getParsedConfigFile. This was only to share the lazy import of tsserver. The existing Jest mocks seem to rely on tsserver being a lazy import. I'm not very familiar with eslint's module import cache-bypassing behavior, so I don't know if there is a performance implication here. The original lazy import is also still present in packages/typescript-estree/tests/lib/createProjectService.test.ts. We could try adding tsserver as a parameter to getParsedConfigFile to remove the extra lazy import if we think it will be a problem.
Switched from a global mock to a doMock inside a beforeAll for testing tsserver.env === undefined case. I've primarily used mocha/chai, vitest, and bun test; so if it there is a cleaner way to do this in jest I can make that update.
All previous unit tests in packages/typescript-estree/tests/lib/createProjectService.test.ts were truly unit tests mocking all calls to tsserver. Wanting to test the extends behavior, some of the tests now call through to tsserver. Since these are really testing integration functionality, please let me know if adjustments are required. These tests also have slightly looser assertions since tsserver adds some metadata. A code comment has been added to mark each.
I did not end up needing to use all of the projectServicesComplex fixture, specifically the project references. It may have future uses, but I can trim the currently unused project references if preferred.

Inline comments work for me on the PR.

…s initial tests

typescript-eslint · 2024-06-08T02:05:26Z

Thanks for the PR, @higherorderfunctor!

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-06-08T02:05:44Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`4478aea`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/66a62e98249f920008d167c1
😎 Deploy Preview	https://deploy-preview-9306--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (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-06-08T02:07:21Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4478aea. 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 1 target

nx run repo-tools:postinstall-script

Sent with 💌 from NxCloud.

…ct-service-extends-issue

codecov · 2024-06-08T21:17:12Z

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.04%. Comparing base (f3dfc0a) to head (ac8c9bc).
Report is 4 commits behind head on v8.

❗ Current head ac8c9bc differs from pull request most recent head 4478aea

Please upload reports for the commit 4478aea to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##               v8    #9306      +/-   ##
==========================================
+ Coverage   88.01%   88.04%   +0.03%     
==========================================
  Files         401      402       +1     
  Lines       13637    13639       +2     
  Branches     3960     3959       -1     
==========================================
+ Hits        12003    12009       +6     
+ Misses       1325     1322       -3     
+ Partials      309      308       -1

Flag	Coverage Δ
unittest	`88.04% <95.83%> (+0.03%)`	⬆️

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

Files	Coverage Δ
...-estree/src/create-program/createProjectService.ts	`95.34% <100.00%> (+1.87%)`	⬆️
...t-estree/src/create-program/useProvidedPrograms.ts	`100.00% <100.00%> (+12.12%)`	⬆️
...t-estree/src/create-program/getParsedConfigFile.ts	`93.75% <93.75%> (ø)`

... and 3 files with indirect coverage changes

… tests

JoshuaKGoldberg

I like the general direction! +1 on extracting out a getParsedConfigFile, that's really useful. 🚀

Requesting changes mostly on reorganization, and the require()ing too. Cheers!

packages/typescript-estree/src/create-program/getParsedConfigFile.ts

packages/typescript-estree/tests/lib/createProjectService.test.ts

packages/typescript-estree/src/create-program/getParsedConfigFile.ts

…ts with review feedback Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

JoshuaKGoldberg · 2024-07-17T03:17:09Z

👋 are you waiting on us for anything @higherorderfunctor?

higherorderfunctor · 2024-07-17T03:21:25Z

👋 are you waiting on us for anything @higherorderfunctor?

Nope! I'll be picking this back up this week / weekend. Had a vacation and some deadlines the last two weeks.

JoshuaKGoldberg · 2024-07-18T03:46:22Z

Great! Let's time box this to a week from now, then - we really want to release v8 as soon as possible.

higherorderfunctor · 2024-07-23T14:20:02Z

Created getParsedConfigFile.test.ts.

Will revert most of the changes to createProjectService.test.ts except for the use of the new function (mocked).
Decide if dropping or creating the complex tests as an integration (probably drop).
Merge in v8 again and resolve conflicts.

Plan to be done by tonight.

…ice-extends-issue

higherorderfunctor · 2024-07-24T01:52:50Z

@JoshuaKGoldberg I will have some time tomorrow to check on the CI for any errors or for any final edits you would like to make. Thanks!

JoshuaKGoldberg

🔥 this is great, thanks!

I only have one teeny nit. It's absolutely not blocking at all. Will leave this open for a bit in case anybody else from the team has time to review.

packages/typescript-estree/src/create-program/getParsedConfigFile.ts

packages/typescript-estree/tests/lib/createProjectService.test.ts

Co-authored-by: Brad Zacher <brad.zacher@gmail.com> Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

higherorderfunctor · 2024-07-25T17:51:29Z

All changes made. Let me know if anything else!

I think the running CI job is stuck, netlify shows complete.

Opened #9641 for the test clean up.

JoshuaKGoldberg

🔥 thanks!

packages/typescript-estree/src/create-program/createProjectService.ts

…vice.ts

refactor(typescript-estree): refactors default project config and add…

69aa3c9

…s initial tests

higherorderfunctor changed the title ~~fix(typescript-estree): Adds support for project services and extended config files~~ fix(typescript-estree): adds support for project services and extended config files Jun 8, 2024

higherorderfunctor added 3 commits June 8, 2024 14:08

test(typescript-estree): fix original failing unit tests

d32c1e5

chore(typescript-estree): cleans up PR submission

148a9d0

chore: merge remote-tracking branch 'upstream/v8' into fix/9205-proje…

8bc4b40

…ct-service-extends-issue

higherorderfunctor added 4 commits June 8, 2024 15:19

chore(typescript-estree): fixes test names to use the new function name

b6718a1

test(typescript-estree): adds additional tests for coverage

bde1a26

test(typescript-estree): fixes test for windows

3315345

test(typescript-estree): fixes test name typo

2859ef9

higherorderfunctor marked this pull request as ready for review June 8, 2024 23:25

higherorderfunctor added 2 commits June 8, 2024 18:04

chore(typescript-estree): minor cleanup to comments and variables

f01fe0e

style(typescript-estree): unifies how compiler options are defined in…

73e2c78

… tests

higherorderfunctor changed the title ~~fix(typescript-estree): adds support for project services and extended config files~~ fix(typescript-estree): adds support for project services using extended config files Jun 10, 2024

This was referenced Jun 13, 2024

Bug: Vue + TypeScript project slowdown with v8 alpha #9312

Closed

fix(typescript-estree): only run projectService setHostConfiguration when needed #9336

Merged

JoshuaKGoldberg added this to the 8.0.0 milestone Jun 20, 2024

JoshuaKGoldberg requested changes Jun 25, 2024

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jun 25, 2024

chore(typescript-estree): updates create-program/getParsedConfigFile.…

4b8c010

…ts with review feedback Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

higherorderfunctor added 2 commits July 19, 2024 12:18

chore: merge upstream/v8

4f0e2de

test(typescript-estree): adds initial getParsedConfigFile tests

abd0095

test(typescript-estree): updates createProjectService tests

c563b8d

higherorderfunctor added 3 commits July 23, 2024 19:24

chore: merge upstream/v8

4d043be

test(typescript-estree): cleans up PR

c7181b9

Merge remote-tracking branch 'upstream/v8' into fix/9205-project-serv…

6d23cb1

…ice-extends-issue

test(typescript-estree): fixes snapshot tests

45551dc

JoshuaKGoldberg approved these changes Jul 24, 2024

View reviewed changes

packages/typescript-estree/src/create-program/getParsedConfigFile.ts Outdated Show resolved Hide resolved

JoshuaKGoldberg added 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge and removed awaiting response Issues waiting for a reply from the OP or another party labels Jul 24, 2024

JoshuaKGoldberg requested a review from bradzacher July 24, 2024 13:15

bradzacher reviewed Jul 24, 2024

View reviewed changes

higherorderfunctor and others added 2 commits July 24, 2024 19:09

style(typescript-estree): applies suggestions from code review

d5f06c5

Co-authored-by: Brad Zacher <brad.zacher@gmail.com> Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

style(typescript-estree): applies manual suggestions from code review

1dd15bf

Co-authored-by: Brad Zacher <brad.zacher@gmail.com> Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

higherorderfunctor mentioned this pull request Jul 25, 2024

Repo(typescript-estree): Build out test cases using using fixtures for project service tests to reduce mocking #9641

Open

fixes(typescript-estree): fixes linting errors

ac8c9bc

JoshuaKGoldberg approved these changes Jul 28, 2024

View reviewed changes

packages/typescript-estree/src/create-program/createProjectService.ts Outdated Show resolved Hide resolved

Update packages/typescript-estree/src/create-program/createProjectSer…

4478aea

…vice.ts

JoshuaKGoldberg merged commit 2a05336 into typescript-eslint:v8 Jul 28, 2024
4 checks passed

JoshuaKGoldberg deleted the fix/9205-project-service-extends-issue branch July 28, 2024 11:42

github-actions bot mentioned this pull request Jul 28, 2024

Bug(typescript-estree): Project services default project not working with extended tsconfigs #9205

Closed

4 tasks

JoshuaKGoldberg mentioned this pull request Jul 28, 2024

Bug: tsconfig.json not respected in new files when multiple tsconfigs include the same directory with projectService enabled #8835

Open

4 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(typescript-estree): adds support for project services using extended config files #9306

fix(typescript-estree): adds support for project services using extended config files #9306

higherorderfunctor commented Jun 8, 2024 •

edited

Loading

typescript-eslint bot commented Jun 8, 2024

netlify bot commented Jun 8, 2024 •

edited

Loading

nx-cloud bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

JoshuaKGoldberg commented Jul 17, 2024

higherorderfunctor commented Jul 17, 2024

JoshuaKGoldberg commented Jul 18, 2024

higherorderfunctor commented Jul 23, 2024

higherorderfunctor commented Jul 24, 2024

JoshuaKGoldberg left a comment

higherorderfunctor commented Jul 25, 2024

JoshuaKGoldberg left a comment

fix(typescript-estree): adds support for project services using extended config files #9306

fix(typescript-estree): adds support for project services using extended config files #9306

Conversation

higherorderfunctor commented Jun 8, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Jun 8, 2024

netlify bot commented Jun 8, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Jun 8, 2024 • edited Loading

☁️ Nx Cloud Report

codecov bot commented Jun 8, 2024 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg commented Jul 17, 2024

higherorderfunctor commented Jul 17, 2024

JoshuaKGoldberg commented Jul 18, 2024

higherorderfunctor commented Jul 23, 2024

higherorderfunctor commented Jul 24, 2024

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

higherorderfunctor commented Jul 25, 2024

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

higherorderfunctor commented Jun 8, 2024 •

edited

Loading

netlify bot commented Jun 8, 2024 •

edited

Loading

nx-cloud bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading