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

Error running eslint --fix when multiple .gts files are present #119

Closed
bendemboski opened this issue Jan 24, 2025 · 6 comments · Fixed by #126
Closed

Error running eslint --fix when multiple .gts files are present #119

bendemboski opened this issue Jan 24, 2025 · 6 comments · Fixed by #126

Comments

@bendemboski
Copy link
Contributor

Repro

  1. npx [email protected] new my-app --pnpm --typescript
  2. cd my-app
  3. echo -e 'const Foo = <template></template>;\nexport default Foo;' > app/components/foo.gts
  4. echo -e '// eslint-disable-next-line prefer-const\nconst Bar = <template></template>;\nexport default Bar;' > app/components/bar.gts
  5. pnpm lint:js:fix

Result

The unused eslint-disable-next-line comment is deleted, but the now-blank line is not removed, and the following error occurs:

TypeError: Cannot read properties of undefined (reading 'sourceFile')
    at isDocumentRegistryEntry (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:142504:18)
    at getDocumentRegistryEntry (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:142580:19)
    at Object.releaseDocumentWithKey (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:142675:19)
    at Object.getOrCreateSourceFileByPath [as getSourceFileByPath] (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:151555:30)
    at tryReuseStructureFromOldProgram (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:126294:59)
    at createProgram (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:125689:23)
    at synchronizeHostDataWorker (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:151491:15)
    at synchronizeHostData (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:151386:7)
    at Object.getProgram (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:151568:5)
    at ConfiguredProject2.updateGraphWorker (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:186518:41)
    at ConfiguredProject2.updateGraph (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:186353:32)
    at ConfiguredProject2.updateGraph (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:187646:24)
    at updateWithTriggerFile (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:188190:11)
    at updateConfiguredProject (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:188198:9)
    at updateProjectFoundUsingFind (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:188133:28)
    at _ProjectService.findCreateOrReloadConfiguredProject (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:190784:62)
    at _ProjectService.tryFindDefaultConfiguredProjectForOpenScriptInfo (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:190819:25)
    at _ProjectService.tryFindDefaultConfiguredProjectAndLoadAncestorsForOpenScriptInfo (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:190977:25)
    at _ProjectService.assignProjectToOpenedScriptInfo (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:190730:27)
    at _ProjectService.openClientFileWithNormalizedPath (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:191088:48)
    at _ProjectService.openClientFile (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/typescript.js:190638:17)
    at openClientFile (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/useProgramFromProjectService.js:113:40)
    at openClientFileAndMaybeReload (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/useProgramFromProjectService.js:118:22)
    at openClientFileFromProjectService (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/useProgramFromProjectService.js:68:20)
    at useProgramFromProjectService (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/useProgramFromProjectService.js:185:9)
    at getProgramAndAST (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:44:100)
    at parseAndGenerateServices (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected]/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:154:11)
    at Object.parseForESLint (/private/tmp/lint-error/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@typescript-eslint/parser/dist/parser.js:101:80)
    at Object.parseForESLint (/private/tmp/lint-error/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected][email protected]_t_stmvikz5ttjhkvxi3gt7qnuzp4/node_modules/ember-eslint-parser/src/parser/gjs-gts-parser.js:55:30)
    at Object.parse (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/languages/js/index.js:270:26)
    at ParserService.parseSync (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/services/parser-service.js:36:33)
    at #flatVerifyWithoutProcessors (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1731:47)
    at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:2062:49)
    at /private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1671:29
    at Array.map (<anonymous>)
    at Linter._verifyWithFlatConfigArrayAndProcessor (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1666:36)
    at Linter._verifyWithFlatConfigArray (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:2138:25)
    at Linter.verify (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1605:61)
    at Linter.verifyAndFix (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:2389:29)
    at verifyText (/private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/eslint.js:327:48)
    at /private/tmp/lint-error/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/eslint/eslint.js:807:40

/private/tmp/lint-error/app/components/foo.gts
  0:0  error  Parsing error: Cannot read properties of undefined (reading 'sourceFile')

✖ 1 problem (1 error, 0 warnings)

 ELIFECYCLE  Command failed with exit code 1.

Analysis

The proximate cause of the error is that isDocumentRegistryEntry() is called with undefined rather than a valid entry. This is happening because releaseDocumentWithKey is being called with a path of /private/tmp/my-app/app/components/bar.mts, but there's no corresponding value in bucket, it only contains

'/private/tmp/my-app/app/app.ts'
'/private/tmp/my-app/app/config/environment.d.ts'
'/private/tmp/my-app/app/router.ts'
'/private/tmp/my-app/app/components/bar.gts'
'/private/tmp/my-app/app/components/foo.gts'
'/private/tmp/my-app/tests/test-helper.ts'
'/private/tmp/my-app/tests/helpers/index.ts'
'/private/tmp/my-app/types/global.d.ts'

So my guess is that the ts-patch.ts stuff this parser does is causing these mts entries to get added somewhere but then not handled properly.

The only other clue that I noticed is that further up the stack is a function called tryReuseStructureFromOldProgram, and it looks like we're hitting an error in the process of trying to find info about files in an old typescript "program" so maybe we can reuse that info in the new "program"? So (wildly guessing here) perhaps we build a typescript program, run the lint-fixing, but then rerun it to see if other linters have new fixes to apply, and the ts-patching this parser does leaves mts entries in the old program that this try-reuse logic trips over.

@bendemboski
Copy link
Contributor Author

Oh, another note -- at @NullVoxPopuli's suggestion I tried removing the prettier plugin from the linting config, but that doesn't affect anything -- we still get this same error.

@bendemboski
Copy link
Contributor Author

Here is a more minimal repro. I have also confirmed that the error only happens if two .gts files are present -- if I delete foo.gts and run pnpm lint:js:fix the error does not occur.

@bendemboski
Copy link
Contributor Author

This appears to be a compatibility issue with typescript-eslint@8. If I downgrade the dependency to v7, the error no longer occurs.

@abeforgit
Copy link

@bendemboski
Oh my god thank you so much for raising this, you've saved me so much time 🙈
For extra context: I was getting the same error from the eslint language server, both in neovim and in vscode, as soon as I made any edit to a .gts file.
So it affects more than just the --fix action.
This on a recently converted v2 addon, using the latest blueprint as of a few days ago

Downgrading to v7 fixed it for me as well!

@mkszepp
Copy link

mkszepp commented Jan 27, 2025

Ah... i have reportes this error also here ember-cli/eslint-plugin-ember#2240

When we use TS <= 5.5 the issue is fixed

@bendemboski
Copy link
Contributor Author

I believe the fix for this issue is to add impliedNodeFormat: mtsSourceFile.impliedNodeFormat, here.

I definitely don't fully grok everything here, but gts files end up with an impliedNodeFormat of undefined and mts files end up with an impliedNodeFormat of 99. The bucket thingies that cache source files cache them by their format, so when we run that syncing logic on an existing mts file we were overwriting its impliedNodeFormat and setting it to undefined, and then the ts code ended up looking in the wrong bucket for it and blowing up.

@NullVoxPopuli if you can help me get a branch of this repo that reproduces the error, I would happily and eagerly test & PR this fix.

bendemboski added a commit to bendemboski/ember-eslint-parser that referenced this issue Jan 28, 2025
In the `sourceFile` objects that `typescript` manages, `gts` files end up with an `impliedNodeFormat` of `undefined` and `mts` files end up with an `impliedNodeFormat` of `99`. The buckets that `typescript` uses to cache the source files are keyed by their format, so when we run this syncing logic on an existing `mts` file we were overwriting its `impliedNodeFormat` and setting it to `undefined`. Perhaps something changed in `typescript-eslint@8` to take advantage of more caching or program reuse, not sure, but the result is that overwriting the `impliedNodeFormat` of the `mts` files that correspond to un-fixed `gts` files causes the error in ember-tooling#119. Adding it to the list of fields to preserve fixes it in all the cases I've run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants