Fix types for gjs extensions with explicit imports by wagenet · Pull Request #134 · ember-tooling/ember-eslint-parser

wagenet · 2025-08-27T23:41:02Z

We already fixed types for extension-less imports but this fixes explicit extensions.

NullVoxPopuli · 2025-09-03T14:19:08Z

src/parser/gjs-gts-parser.js

+      );
+      return multiValueResolver(values, 'project');
+    } else {
+      // For allowArbitraryExtensions - return first found value


why is this comment here? the code doesn't do anything with any specific option

Looks like something that got left behind on accident.

NullVoxPopuli · 2025-09-03T14:28:14Z

src/parser/gjs-gts-parser.js

  if (tsconfigPaths.length > 0) {
-    const allowJsValues = tsconfigPaths.map((cfg) => parseAllowJsFromTsconfig(cfg, rootDir));
-    return resolveAllowJs(allowJsValues, 'project');
+    if (multiValueResolver) {


this multi-value resolver feels a bit silly.
It's only used by getAllowJs, and no other code cares - so this makes the factoring of this function feel a bit wierd.

Like, the tasks of collecting tsconfigs feels different from getting the value from a collection of tsconfigs.

at the same time, I almost think having a whole control from for multiValueResolver is silly, and we should get rid of it, but instead have a different mechanism for warning when tsconfigs have conflicting values.

atm, this feels like its trying to be too generic, but is actually a very specific use case, so it's an unneeded abstraction (so far).

I would get rid of the multiValueResolver entirely.

And instead, have a hard-coded list of properties that we want to warn about conflicting values for.

Then, in the (currently) else branch below, do:

if (WARN_ON_CONFLICT.has(property)) { printWarnIfConflict(tsconfigPaths, rootDir, property); } for (const tsconfigPath of tsconfigPaths) { const result = parseCompilerOptionFromTsconfig(tsconfigPath, rootDir, property); if (result !== undefined) return result; }

Yeah, this was probably the AI over optimizing.

NullVoxPopuli · 2025-09-03T14:30:20Z

src/parser/gjs-gts-parser.js

+        const programAllowArbitraryExtensions =
+          result.services.program.getCompilerOptions?.()?.allowArbitraryExtensions;
+        if (
+          !allowArbitraryExtensionsWasSet &&


how does this situation happen?

I'm not sure. I wish I knew! I've seen it actually happen with allowGjs.

NullVoxPopuli · 2025-09-03T16:08:37Z

src/parser/ts-patch.js

        }
-        if (fileName.endsWith('.gts') || (allowGjs && fileName.endsWith('.gjs'))) {
+        // Only transform template files, not declaration files
+        if (


does this need to account for .d.gjs.ts?

No, but we should clean it up. Since a file can't both end with .gts and also end with .d.ts.

NullVoxPopuli · 2025-09-03T16:09:17Z

test-projects/configs/flat-ts/src/api-consumer.ts

is this a relevant test file?

I suppose ts importing gjs is good, ya

NullVoxPopuli · 2025-09-03T17:07:02Z

test-projects/configs/flat-ts/tsconfig.json

+
+      Allow importing files with arbitrary extensions like .d.gjs.ts
+    */
+    "allowArbitraryExtensions": true,


should flat-ts actually be duplicated to a different folder (flat-ts-arbitrary-extensions) -- since we have control flows that run when this is false, I think we should retain that, and create a separate test project for this use case -- all the new files can be moved in to that new project

NullVoxPopuli · 2025-09-03T17:07:11Z

test-projects/gts/src-fixable/test-fix.gts

@@ -1,2 +1,2 @@
-const Bar = () => <template></template>;
+const Bar = () => {return <template></template>};


AI! :shakes-fist:

NullVoxPopuli · 2025-09-03T17:11:26Z

tests/parser.test.js

+    expect(importDeclarations[1].specifiers[0].imported.name).toBe('ApiClient');
+    expect(importDeclarations[1].specifiers[1].imported.name).toBe('DefaultClient');
+
+    // Verify that our extension replacement transforms .gjs to .mjs for TypeScript processing


why combine these assertions with the import verification assertions in this test?

feels like testing replaceExtensions is sort of a different thing altogether than the importDeclarations testing above

NullVoxPopuli · 2025-09-03T17:11:37Z

tests/parser.test.js

+    const { patchTs, replaceExtensions } = require('../src/parser/ts-patch');
+    patchTs({ allowGjs: true });
+
+    const transformedCode = replaceExtensions(codeWithComplexGjsImports);


wagenet · 2025-09-03T17:43:24Z

src/parser/ts-patch.js

          (!fileName.endsWith('.d.ts') && fileName.endsWith('.ts')) ||
-          fileName.endsWith('.gts') ||
-          (allowGjs && fileName.endsWith('.gjs'))
+          (fileName.endsWith('.gts') && !fileName.endsWith('.d.ts')) ||


We should clean this up too.

mkszepp · 2025-11-01T06:51:07Z

~~This should fix bug no. 2 which we have in our app since july... typed-ember/glint#915 (comment)~~

Edit:
With Typescript projects there was not working explicit imports with .gts
The fix in my case is, that we need to add in parserOptions the property project: true see ... Not sure if this solves also the issue in gjs

NullVoxPopuli · 2025-11-12T14:18:19Z

probably needs rebase?

wagenet force-pushed the fix-gjs-more branch from 1705767 to 2d84c22 Compare August 29, 2025 18:02

wagenet marked this pull request as ready for review August 29, 2025 20:12

NullVoxPopuli requested changes Sep 3, 2025

View reviewed changes

wagenet commented Sep 3, 2025

View reviewed changes

mkszepp mentioned this pull request Nov 1, 2025

[Pre-release / Release v2] Unsafe assignment of an error typed value. when import is from an other gts file typed-ember/glint#915

Closed

This was referenced Nov 12, 2025

Upgrade pnpm re roll lockfile fix gjs more test merge #139

Closed

Fix gjs more #143

Closed

wagenet added 7 commits January 9, 2026 17:02

Fix filename typo

15b3a9e

Handle types for gjs imports with explicit extension

d191b5c

Clean up

5d74f00

Fix hardcoded paths

2ffc4ed

Actually respsect allowArbitraryExtensions

1ba7a20

More clean up

76e97df

Clean up some

9eea16c

NullVoxPopuli force-pushed the fix-gjs-more branch from e51759d to 9eea16c Compare January 9, 2026 22:02

		@@ -1,2 +1,2 @@
		const Bar = () => <template></template>;
		const Bar = () => {return <template></template>};

Conversation

wagenet commented Aug 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkszepp commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NullVoxPopuli commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkszepp commented Nov 1, 2025 •

edited

Loading