feat(typescript-estree): add EXPERIMENTAL_useProjectService option to use TypeScript project service #6754

JoshuaKGoldberg · 2023-03-24T12:21:11Z

PR Checklist

Addresses an existing open issue: fixes Enhancement: Try using tsserverlibrary's ProjectService to handle type-aware mode #6575
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Creates an experimental ready-to-try-only-if-you-dare prototype implementation of #6575: using TypeScript's "project service" / server to create programs instead of our existing manual project creation. Doing so brings the "create program(s) for source files" part of typed linting towards how editors behave:

Using the nearest tsconfig.json for each file (similar to parserOptions.project: true)
Using a default ("inferred") tsconfig.json if one is not found applicable to the file
- This would overcome Docs: Typed Linting guide causes ESLint complaints in .eslintrc.(c)js #6826 with no additional user configuration, hooray!

// .eslintrc.js
module.exports = {
  extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended-type-checked',
  ],
  plugins: ['@typescript-eslint'],
  parser: '@typescript-eslint/parser',
  parserOptions: {
    EXPERIMENTAL_useProjectService: true,
  },
  root: true,
};

Technical Details

In essence, this PR:

Adds an EXPERIMENTAL_useProjectService option to typescript-estree
- Setting process.env.TYPESCRIPT_ESLINT_EXPERIMENTAL_TSSERVER can also enable the option
Adjusts createParserSettings to call to a new createProjectService function lazily when the experimental option is enabled (thereby setting parseSettings.EXPERIMENTAL_projectService)
Adjusts getProgramAndAST to use that project service if it exists, with a new useProgramFromProjectService
Massages internal tests to always use absolute (or failing that, relative to project root) paths for files if possible, as the project service expects that (is this phrasing accurate?)
Skips tests that aren't applicable anymore when the project service is in use

See https://github.com/JoshuaKGoldberg/repros/tree/cbe9d9c9b19c09b085a0be2024221b2d05753cc8 for a reproduction case showing this working.

Performance Comparison

Complete Project Linting

for branchName in v6 create-project-service; do
    git checkout $branchName
    yarn
    hyperfine "yarn lint --skip-nx-cache" > $branchName.txt
done 

hyperfine "TYPESCRIPT_ESLINT_EXPERIMENTAL_TSSERVER=true yarn lint --skip-nx-cache" > enabled.txt

`v6`	#6754 baseline	#6754 enabled	#6754 Δ
`62.976 s ± 0.479 s`	`65.383 s ± 0.226 s`	`31.169 s ± 0.401 s`	-52% 🟢

Incremental Project Linting

for glob in "eslint-plugin/src/rules/*.ts" "typescript-estree/src/create-program/*.ts" "website/**/*.tsx"; do
    for setting in false true; do
        hyperfine "TYPESCRIPT_ESLINT_EXPERIMENTAL_TSSERVER=$setting npx eslint --config .eslintrc.js packages/$glob"
    done
done

Glob	Baseline	Enabled	Δ
`eslint-plugin/src/rules/*.ts`	`8.552 s ± 0.100 s`	`9.525 s ± 0.084 s`	+11% ❌
`packages/typescript-estree/src/create-program/*.ts`	`7.162 s ± 0.079 s`	`2.153 s ± 0.007 s`	-70% 🟢
`packages/website/*/.tsx`	`9.955 s ± 0.080 s`	`3.714 s ± 0.029 s`	-63% 🟢

typescript-eslint · 2023-03-24T12:21:13Z

Thanks for the PR, @JoshuaKGoldberg!

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 · 2023-03-24T12:21:16Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`7e74202`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/64b3315b3783730008dbc7af
😎 Deploy Preview	https://deploy-preview-6754--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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

packages/typescript-estree/src/parser.ts

nx-cloud · 2023-03-24T12:24:36Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7e74202. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 35 targets

Sent with 💌 from NxCloud.

packages/typescript-estree/src/parser.ts

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

codecov · 2023-04-18T17:43:03Z

Codecov Report

Merging #6754 (7e74202) into main (606a52c) will decrease coverage by 0.11%.
The diff coverage is 62.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6754      +/-   ##
==========================================
- Coverage   87.57%   87.47%   -0.11%     
==========================================
  Files         377      379       +2     
  Lines       13201    13230      +29     
  Branches     3901     3906       +5     
==========================================
+ Hits        11561    11573      +12     
- Misses       1261     1279      +18     
+ Partials      379      378       -1

Flag	Coverage Δ
unittest	`87.47% <62.50%> (-0.11%)`	⬇️

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

Impacted Files	Coverage Δ
...escript-estree/src/useProgramFromProjectService.ts	`16.66% <16.66%> (ø)`
...t-estree/src/create-program/useProvidedPrograms.ts	`84.84% <66.66%> (ø)`
...pt-estree/src/parseSettings/createParseSettings.ts	`81.81% <71.42%> (+0.18%)`	⬆️
...-estree/src/create-program/createProjectService.ts	`80.00% <80.00%> (ø)`
packages/eslint-plugin-tslint/src/rules/config.ts	`97.77% <100.00%> (+0.05%)`	⬆️
packages/typescript-estree/src/clear-caches.ts	`100.00% <100.00%> (ø)`
...-estree/src/create-program/createProjectProgram.ts	`97.56% <100.00%> (-0.12%)`	⬇️
.../src/create-program/getWatchProgramsForProjects.ts	`82.12% <100.00%> (-2.71%)`	⬇️
...ges/typescript-estree/src/create-program/shared.ts	`77.50% <100.00%> (ø)`

... and 1 file with indirect coverage changes

packages/typescript-estree/src/useProgramFromProjectService.ts

JoshuaKGoldberg · 2023-06-13T20:25:26Z

Re-requesting review from @bradzacher because I applied a couple of small touchups & answered a question. But otherwise I feel comfortable merging this after we get v6 out the door (just to be safe & not conflict with the v6 release). Woop woop! 🚀

bradzacher

ritschwumm · 2023-07-10T19:29:18Z

may i ask what happened with this PR - is it merged now, or had it to be abandoned?

JoshuaKGoldberg · 2023-07-10T19:32:01Z

This was unintentionally auto-closed when we merged the v6 branch 🙃 it'll be ~~recreated~~ reopened.

The base branch was changed.

JoshuaKGoldberg · 2023-07-10T21:17:10Z

Now that v6 is merged into main, I'm planning on merging this as well some time in the next 1-2 weeks. Depending on whether there are any major issues reported with 6.0.0:

If there are, we'll fix them in next week's release (July 17th) and I'll plan to merge this this in time for the following week (July 24th)
If there aren't, I'll plan to merge this in time for next week's release (July 17th)

Note that we're keeping the EXPERIMENTAL prefix on the option name for now. It's very new and scary. We're also going to want to think a bit more deeply on how the parserOptions type shape should look.

FYI @jakebailey - we're moving quickly this month! ⚡

JoshuaKGoldberg · 2023-07-15T23:30:59Z

https://github.com/typescript-eslint/typescript-eslint/milestone/10 has a small enough number of bugs -and no major architectural issues- reported that I feel comfortable merging this in. Hooray! 🚀

(knocking on wood, fingers crossed 🤞)

JoshuaKGoldberg · 2023-07-16T00:27:18Z

This change terrifies me.

bradzacher · 2023-07-17T01:19:47Z

.github/workflows/ci.yml

+  unit_tests_tsserver:
+    name: Run Unit Tests with Experimental TSServer
+    needs: [build]
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        package:
+          [
+            'eslint-plugin',
+            'eslint-plugin-internal',
+            'eslint-plugin-tslint',
+            'typescript-estree',
+          ]
+    env:
+      COLLECT_COVERAGE: false
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+      - name: Install
+        uses: ./.github/actions/prepare-install
+        with:
+          node-version: 18
+      - name: Build
+        uses: ./.github/actions/prepare-build
+      - name: Run unit tests for ${{ matrix.package }}
+        run: npx nx test ${{ matrix.package }} --coverage=false
+        env:
+          CI: true
+          TYPESCRIPT_ESLINT_EXPERIMENTAL_TSSERVER: true


This was a great thing to add!