Errors in .mdx files with mdx.checkMdx in tsconfig.json appear at end of file

[karlhorky]

Companion issue to mdx-js/mdx-analyzer#451

Describe the bug

Usage with [email protected] along with [email protected] leads to:

1. All errors in .mdx files reported at end of file
2. Selecting the IntelliSense auto-import entry for ZoomImage adds the new import to the bottom of the code

Disabling [email protected] leads to errors being reported in the correct locations

To Reproduce

Steps to reproduce the behavior:

1. Clone the reproduction at https://github.com/karlhorky/repro-mdx-analyzer-missing-auto-import-intellisense
2. Install the package.json dependencies
3. Install the 2 recommended extensions [email protected] and [email protected] (both enabled)
4. In app/message.mdx, observe that the errors are reported at a one-character red squiggly at the end of the file, instead of in the correct locations

Kapture.2024-06-28.at.17.33.21.mp4

Expected behavior

1. Error reported in correct location
2. Selecting the IntelliSense auto-import entry for ZoomImage should add the new import to the top of the code

Screenshots

Already included above

Desktop (please complete the following information):

• OS: macOS
• Version Sonoma 14.5 (23F79)

VS Code version information:

Version: 1.91.0-insider
Commit: aea213b7fcc7de5c24ad797ac1af209b159d451f
Date: 2024-06-28T06:02:58.030Z
Electron: 29.4.0
ElectronBuildId: 9728852
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Darwin arm64 23.5.0

Additional context

--

[Viijay-Kr]

Seems interesting. Initial thoughts. 1.The extension should not be activated for MDX language unless they are considered as JSX (courtesy MDX extension)
2. Do yo see any error on hover ?

[karlhorky]

1.The extension should not be activated for MDX language unless they are considered as JSX (courtesy MDX extension)

Good to know, maybe that's useful information for tracking down this problem.

2. Do yo see any error on hover ?

Yes, it's visible in the video

[Viijay-Kr]

@karlhorky Did you notice this , ? the extension never gets activated like I presumed

[Viijay-Kr]

@karlhorky I modified the langauge service plugin based on the volar PR

Viijay-Kr/typescript-cleanup-defs#4 .

This fixes the problem for me . I tested it locally . If you wish to test it ? clone the repo , link the package in react-ts-css project and run the extension locally , the errors appear at the right location

[karlhorky]

With "the Volar PR", you mean volarjs/volar.js#216, right?

I wonder if this PR will actually resolve the issue, once it is pulled in to MDX Analyzer and a new version of MDX Analyzer released...

This fixes the problem for me . I tested it locally . If you wish to test it ? clone the repo , link the package in react-ts-css project and run the extension locally , the errors appear at the right location

Sounds great though! Maybe it's best to have it resolved in both extensions anyway. I will try to find some time in the next days to test this...

[Viijay-Kr]

I wonder if this PR will actually resolve the issue, once it is pulled in to MDX Analyzer and a new version of MDX Analyzer released...

I don't follow this 🤔. I haven't gone over mdx analyzer project. Does MDX analyzer process all the typrescript server plugins installed by other extensions ? even if it did I tested it using the setup you recommended , using the mdx analyzer extension. So please enlighten me If I am missing something

With "the Volar PR", you mean volarjs/volar.js#216, right?

Yes thats right.

[karlhorky]

I don't follow this 🤔. I haven't gone over mdx analyzer project. Does MDX analyzer process all the typrescript server plugins installed by other extensions ? even if it did I tested it using the setup you recommended , using the mdx analyzer extension. So please enlighten me If I am missing something

Ah I'm not sure you're missing anything - I was just wondering if fixing it in MDX Analyzer would also fix it in other extensions. But fixing in both places (also in React CSS modules) is probably better and maybe also even necessary.

[karlhorky]

I tested it locally . If you wish to test it ? clone the repo , link the package in react-ts-css project and run the extension locally , the errors appear at the right location

I would try to test this out today - just having trouble understanding the instructions.

Is there a way to apply the patch manually (eg. from the Files changed in the PR) by editing the extension files locally on my machine? (instead of building/linking anything or installing a different local version of the extension) Something similar to the workflow of editing node_modules by hand?

[karlhorky]

Ah yeah, I can edit the files in place by editing this file:

code ~/.vscode/extensions/viijay-kr.react-ts-css-3.2.0/node_modules/typescript-cleanup-definitions/index.js

This was the change I applied (based on Viijay-Kr/typescript-cleanup-defs#4):

-            const proxy = Object.create(null);
+            const proxy = new Proxy(info.languageService, {
+                get(target, p, receiver) {
+                    return Reflect.get(target, p, receiver)
+                },
+                set(target, p, newValue, receiver) {
+                    return Reflect.set(target, p, newValue, receiver)
+                },
+            });

This indeed resolves the problem:

Before

After