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

Enable parserOptions.allowAutomaticSingleRunInference #890

Open
JoshuaKGoldberg opened this issue Dec 24, 2023 · 8 comments
Open

Enable parserOptions.allowAutomaticSingleRunInference #890

JoshuaKGoldberg opened this issue Dec 24, 2023 · 8 comments

Comments

@JoshuaKGoldberg
Copy link
Contributor

Coming over from https://twitter.com/andhaveaniceday/status/1738986013320794313: typescript-eslint has a relatively undocumented allowAutomaticSingleRunInference feature that in many cases speeds up single-run linting. That includes CI.

Because eslint/rfcs#102 wasn't accepted (😞) and one last bug with it has yet to be explained (typescript-eslint/typescript-eslint#3851), we've been cautious to enable it by default in typescript-eslint. But as @jakebailey noted, [DT doesn't] have project references and it's all single run outside of the editor.

@jakebailey
Copy link
Member

jakebailey commented Dec 24, 2023

I misspoke; we do run in the editor, I actually just meant that the big single run is the "slow" run. Does this negatively impact the editor? Should this be set in the runner instead?

@JoshuaKGoldberg
Copy link
Contributor Author

Does this negatively impact the editor?

I don't believe it does, no. @bradzacher can probably keep me honest - but to my knowledge the only known issue is the mysterious one from the case that uses project references.

@bradzacher
Copy link

Note that if you've got custom scripts to run eslint we won't be able to infer it as a CLI run and so the option will do nothing. You'll need to set the env var to force it on for that case.

@jakebailey
Copy link
Member

What's the env var? I must have missed it trying to look through the docs.

@bradzacher
Copy link

TSESTREE_SINGLE_RUN=true should do it for you

@jakebailey
Copy link
Member

Thanks! I think we should probably just set that right at the start of dtslint's main entrypoint, and skip the setting (given it won't work). When I (or someone else) gets a chance, I'll be curious to benchmark it.

I also want to try the ProjectService, though I suspect that there will be some OOMing because the functionality to ask it to drop files is probably not there yet.

@jakebailey
Copy link
Member

So, it turns out that we already do this:

process.env.TSESTREE_SINGLE_RUN = "true";

Removing that, then benchmarking reenabling it, shows that it unfortunately does not help:

Benchmark 1: pnpm test gensync
  Time (mean ± σ):      5.412 s ±  0.022 s    [User: 8.465 s, System: 0.588 s]
  Range (min … max):    5.372 s …  5.435 s    10 runs
 
Benchmark 2: TSESTREE_SINGLE_RUN=true pnpm test gensync
  Time (mean ± σ):      5.444 s ±  0.050 s    [User: 8.517 s, System: 0.599 s]
  Range (min … max):    5.374 s …  5.521 s    10 runs
 
Summary
  'pnpm test gensync' ran
    1.01 ± 0.01 times faster than 'TSESTREE_SINGLE_RUN=true pnpm test gensync'

So, I guess this issue can be closed as we're already enabling it... but is it suspicious that it doesn't actually help?

@JoshuaKGoldberg
Copy link
Contributor Author

Heh, maybe the issue should be tracking why it doesn't actually help?

But we're also planning on enabling it by default in v7 (typescript-eslint/typescript-eslint#8121). And this issue isn't very high on my priority queue to investigate. So I'm probably not going to be taking action soon tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants