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

Bug: EXPERIMENTAL_useProjectService does not respect extraFileExtensions #8899

Open
4 tasks done
alfredringstad opened this issue Apr 11, 2024 · 2 comments · May be fixed by #9051
Open
4 tasks done

Bug: EXPERIMENTAL_useProjectService does not respect extraFileExtensions #8899

alfredringstad opened this issue Apr 11, 2024 · 2 comments · May be fixed by #9051
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Milestone

Comments

@alfredringstad
Copy link

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

When linting a project containing e.g. svelte files, you need to provide extraFileExtensions: ['.svelte'] to make the parser use the correct tsconfig for the specific file. This works well with project: true, since that path ends up in createWatchProgram where we provide the extraFileExtensions option to ts.

However, if we switch EXPERIMENTAL_useProjectService on, we end up in a code path where the extraFileExtensions are not passed to ts. In my reproduction case this is surfaced by the fact that ts will then use the default compiler options, which transpiles the code back to ES5, instead of using the correct tsconfig.

While this reproduction example is using Svelte, the same issue should apply to other cases where extraFileExtensions would be used (such as when using Vite).

Reproduction Repository Link

https://github.com/alfredringstad/typescript-eslint-svelte-use-project-service-bug-reproduction

Repro Steps

  1. clone the repo
  2. npm i
  3. npm run lint

Switch EXPERIMENTAL_useProjectService to false in eslint.config.js and run npm run lint to see how it works well without the project service.

Versions

package version
@typescript-eslint/parser 7.6.0
@typescript-eslint/typescript-estree 7.6.0
TypeScript 5.4.5
ESLint 8.57.0
@alfredringstad alfredringstad added bug Something isn't working triage Waiting for maintainers to take a look labels Apr 11, 2024
@alfredringstad
Copy link
Author

alfredringstad commented Apr 11, 2024

I'm not very familiar with the codebase, so I can't really tell if it makes sense to do this at this specific place, but here's a suggested fix:

// packages/typescript-estree/src/useProgramFromProjectService.ts
import { ScriptKind } from 'typescript';

export function useProgramFromProjectService(
  { allowDefaultProjectForFiles, service }: ProjectServiceSettings,
  parseSettings: Readonly<MutableParseSettings>,
  hasFullTypeInformation: boolean,
): ASTAndDefiniteProgram | undefined {
  // We don't canonicalize the filename because it caused a performance regression.
  // See https://github.com/typescript-eslint/typescript-eslint/issues/8519
  const filePathAbsolute = absolutify(parseSettings.filePath);
  log(
    'Opening project service file for: %s at absolute path %s',
    parseSettings.filePath,
    filePathAbsolute,
  );

  if (parseSettings.extraFileExtensions.length) {
    service.setHostConfiguration({
      extraFileExtensions: parseSettings.extraFileExtensions.map(extension => ({
        extension,
        isMixedContent: false,
        scriptKind: ScriptKind.Deferred
      })),
    });
  }

  const opened = service.openClientFile(
    filePathAbsolute,
    parseSettings.codeFullText,
    /* scriptKind */ undefined,
    parseSettings.tsconfigRootDir,
  );
  /* ... */
}

I'm also not sure if setting scriptKind: ScriptKind.Deferred and isMixedContent: false are sane defaults.

@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Apr 11, 2024
@JoshuaKGoldberg
Copy link
Member

I'm not very familiar with the codebase, ...
(proceeds to post an exact working solution)

That got a chuckle out of me 🙂. I'm in favor of this - would want a review from someone else in @typescript-eslint/triage-team too, but I think this makes a ton of sense. Thanks for trying out the project service & posting such an informative issue+repro!

@bradzacher bradzacher added package: typescript-estree Issues related to @typescript-eslint/typescript-estree accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants