mdx-js / mdx-analyzer

MDX extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=unifiedjs.vscode-mdx
MIT License
349 stars 21 forks source link

Errors in `.mdx` files with `mdx.checkMdx` appear at end of file #451

Closed karlhorky closed 3 months ago

karlhorky commented 5 months ago

Initial checklist

Affected packages and versions

MDX VS Code Extension v1.8.9

Link to runnable example

No response

Steps to reproduce

Reproduction repo: https://github.com/karlhorky/repro-mdx-analyzer-missing-auto-import-intellisense

  1. Set up the app-dir-mdx example from Remco using the content below

(content contains intentional TypeScript errors)

mdx-components.tsx

import { MDXComponents } from 'mdx/types';
import { HTMLAttributes } from 'react';

type ZoomImageProps = {
  alt: string;
};

function ZoomImage(props: ZoomImageProps) {
  return <div>www</div>;
}

function CustomCodeSandBox(props: HTMLAttributes<HTMLDivElement>) {
  return (
    <div {...props} />
  );
}

const components = {
  ZoomImage,
  CustomCodeSandBox,
} satisfies MDXComponents;

declare global {
  type MDXProvidedComponents = typeof components;
}

export function useMDXComponents(): MDXProvidedComponents {
  return components;
}

app/message.mdx

<ZoomImage src="/logo.svg" alt="Logo" />

<CustomCodeSandBox template="react" />

tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "lib": ["dom", "dom.iterable", "esnext"],
    "allowJs": true,
    "skipLibCheck": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noEmit": true,
    "esModuleInterop": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "jsx": "preserve",
    "incremental": true,
    "plugins": [
      {
        "name": "next"
      }
    ]
  },
  "mdx": {
    "checkMdx": true
  },
  "include": [
    "next-env.d.ts",
    "**/*.mdx",
    "**/*.ts",
    "**/*.tsx",
    ".next/types/**/*.ts"
  ],
  "exclude": ["node_modules"]
}
  1. Observe the errors from the MDX VS Code extension appearing on a 1-character red squiggly the end of the file in VS Code instead of on the respective lines where the errors occur: Screenshot 2024-06-27 at 19 48 42

Expected behavior

In the screenshot above, I would expect that the 1st error would appear on line 47, covering the whole line, and the 2nd error would appear on line 49, covering the whole line

Actual behavior

In the screenshot above, the errors from the MDX VS Code extension appear on a 1-character red squiggly the end of the file in VS Code instead of on the respective lines where the errors occur

Runtime

No response

Package manager

No response

OS

macOS

Build and bundle tools

Next.js

karlhorky commented 5 months ago

Interesting, it appears that the highlighting is in the correct location if I disable these 2 extensions 🤔

Screenshot 2024-06-27 at 23 06 27

karlhorky commented 5 months ago

Funny enough, if I disable these 2 extensions, then that also disables the IntelliSense to auto-import into the file:

Extensions enabled

Screenshot 2024-06-27 at 23 07 38

Screenshot 2024-06-28 at 10 45 42

(although it is weird that the import appears at the end of the file... I guess it could be related to the error being reported at the end though)

Extensions disabled

Screenshot 2024-06-27 at 23 07 30

...or maybe IntelliSense to auto-import is not a feature of MDX Analyzer? 🤔


There is also another IntelliSense issue below with vscode-styled-components

However this appears to exhibit the opposite behavior - according to users in 413, enabling these extensions apparently caused IntelliSense suggestions to disappear on older versions (whereas I'm seeing IntelliSense suggestions only appear when the extensions are enabled):

remcohaszing commented 5 months ago

I’m closing this in favour of upstream issue https://github.com/styled-components/vscode-styled-components/issues/448.

I’m not seeing any problems while using viijay-kr.react-ts-css.

github-actions[bot] commented 5 months ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

github-actions[bot] commented 5 months ago

Hi team! Could you describe why this has been marked as external?

Thanks, — bb

karlhorky commented 5 months ago

Ok, thanks for reporting!

I’m not seeing any problems while using viijay-kr.react-ts-css.

I think this may be only caused by certain settings in React CSS modules by @Viijay-Kr (viijay-kr.react-ts-css)

eg. here are my settings:

  "reactTsScss.autoComplete": true,
  "reactTsScss.baseDir": "src",
  "reactTsScss.cleanUpDefs": [
    "node_modules/vite/client.d.ts",
    "node_modules/next/types/global.d.ts",
    "node_modules/react-scripts/lib/react-app.d.ts"
  ],
  "reactTsScss.codelens": true,
  "reactTsScss.cssAutoComplete": true,
  "reactTsScss.cssDefinitions": true,
  "reactTsScss.cssSyntaxColor": true,
  "reactTsScss.definition": true,
  "reactTsScss.diagnostics": true,
  "reactTsScss.peekProperties": true,
  "reactTsScss.references": true,

https://github.com/mdx-js/mdx-analyzer/assets/1935696/84d39fb9-26de-469b-8b66-ba0acfc6527c

remcohaszing commented 5 months ago

I’m just not seeing it.

image

Possibly another extension is the culprit. All TypeScript plugins interact with each other and can accidentally break other plugins. I even recall one plugin recovered issues from another at some point. Perhaps another extension is the culprit.

The following command lists all VSCode extensions that register a TypeScript plugin.

grep -l '"typescriptServerPlugins"' ~/.vscode/extensions/*/package.json
karlhorky commented 5 months ago

Hm, I can repro with only unifiedjs.vscode-mdx@1.8.9 and viijay-kr.react-ts-css@3.2.0

I'll create a reproduction.

remcohaszing commented 5 months ago

FWIW this is how I’m testing this:

karlhorky commented 5 months ago

Here's the reproduction (also shows the missing auto-import IntelliSense - I opened https://github.com/mdx-js/mdx-analyzer/issues/452 for that):

Extensions: unifiedjs.vscode-mdx@1.8.9 and viijay-kr.react-ts-css@3.2.0

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

Repo: https://github.com/karlhorky/repro-mdx-analyzer-missing-auto-import-intellisense

https://github.com/mdx-js/mdx-analyzer/assets/1935696/1be8f03f-d318-4dc2-8b15-682310f1f11c

karlhorky commented 5 months ago

Interesting, it appears that the highlighting is in the correct location if I disable these 2 extensions 🤔

Companion issues have been reported in these two VS Code extensions now:

remcohaszing commented 5 months ago

I’m re-opening this. A change was made upstream in https://github.com/volarjs/volar.js/pull/216, which fixes compatibility with a number of existing TypeScript plugins. Let’s track here until that change has made its way into MDX analyzer.

github-actions[bot] commented 5 months ago

Hi team! I don’t know what’s up as there’s no phase label. Please add one so I know where it’s at.

Thanks, — bb

github-actions[bot] commented 5 months ago

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

jasonwilliams commented 5 months ago

Thanks for raising the detailed issue, sadly I've had to archive https://github.com/styled-components/vscode-styled-components due to having no contributors for quite some time. It's unlikely the issue will be fixed there any time soon, I don't know if there's a replacement extension but until then it will remain the way it is.

karlhorky commented 4 months ago

I’m re-opening this. A change was made upstream in volarjs/volar.js#216, which fixes compatibility with a number of existing TypeScript plugins. Let’s track here until that change has made its way into MDX analyzer.

@remcohaszing Nice, thanks!

For anyone following along, considering that https://github.com/volarjs/volar.js/pull/216 was released in v2.4.0-alpha.1, I guess the version that will probably have the change will be v2.4.0:

Once that version has been released, it can be bumped in MDX Analyzer too.

github-actions[bot] commented 3 months ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

karlhorky commented 3 months ago

@johnsoncodehk @remcohaszing Thanks for the update to Volar 2.4 in PR #460 and the publish of unifiedjs.vscode-mdx@1.8.10 🙌

The problem is half fixed: I tried out the new version with the reproduction repo from the original issue description and:

  1. Correct error locations are used with the (deprecated) Styled Components VS Code extension
  2. Incorrect error locations (end of file) still appear with the React CSS modules VS Code extension (related issue: https://github.com/Viijay-Kr/react-ts-css/issues/160)

https://github.com/user-attachments/assets/ab2edde5-1827-40f7-9f74-606eb4a8cc48

Viijay-Kr commented 3 months ago

@karlhorky Re posting here for visibility

This PR https://github.com/Viijay-Kr/typescript-cleanup-defs/pull/5 adapts similar changes done in volar project . Hopefully this fixes the issue fully

karlhorky commented 3 months ago

@Viijay-Kr thanks for the publish of viijay-kr.react-ts-css@3.2.4, I can confirm the MDX errors are in the correct positions again! 🎉

https://github.com/user-attachments/assets/2e204850-b517-4873-962a-a65dafcf2ff4