Intellisense priority issues breaks Go to definition

[habovh]

Hi,

I've been using this extension really fine on JS projects. It's great and really useful!

However on TS projects, it seems this extension is "fighting" for priority against the TS Intellisense provider. Unfortunately, the TS provider trumps this extension, and when using "Go to definition", it leads to the very generic type definition used to make the compiler happy instead of the actual class in the style module file.

This is rather annoying.

I've never developed a language server extension, but I was wondering if it was possible to increase priority of the suggestions/sources provided by this extension, so that even in case Intellisense matches 2 different definitions, the "chosen" one will always be the one from the extension?

As I've tried to work this out, I discovered that enabling the extension after the JS/TS language features are initialised, then the Go to definition works great. However when I quit and reopen VSCode, the default definition is reverted back to the generic TS one.

Note: I tried all of this without any TS plugin, using VSCode's bundled TS runtime, and on a Next.js project.

[iChenLei]

Yep, my friend(same company employee) encounted the same issue, but I have no idea to fix or improve ux. My friend's scenario is:

declare module '*.module.css' {
  const content: { [className: string]: string }
  export default content; 
}

A possible workaround is

declare module '*.module.css' {
-  const content: { [className: string]: string }
+ const content any;
  export default content; 
}

And then TS will give up type Intellisense. ( I checked it on my macbook

[habovh]

Ooh that's a clever trick! Makes sense that Intellisense would favour anything over any. Thanks for this hack, I wish it wasn't required however.

The "priority" is not great either with autocomplete. It's quite weird the extension is not able to communicate to VSCode that its autocompletion are 100% relevant and should be placed before random stuff.

[karlhorky]

Also just encountered this in a Next.js project (which has CSS modules types in the global types that come along with it in node_modules/next/types/global.d.ts):

The other definition from global.d.ts is not very useful, it's just the generic type definition, including a string. I guess this would not be a desirable destination to use Go to Definition with:

Altering the global.d.ts types file in node_modules to use any does avoid this problem, but I think requiring users to edit files in node_modules is probably not a good solution.

Is there any API in VS Code to remove / invalidate some other definition from within the extension? Or maybe even just re-defining the types to use any in the types for the extension?

[iChenLei]

Is there any API in VS Code to remove / invalidate some other definition from within the extension?

---

@karlhorky I don't know, I can't found any useful info from the official api documentation for this topic.

[karlhorky]

Edit: my post below appears to be not entirely accurate, looks like there is indeed a way to edit the autocomplete definitions that appear: #63 (comment)

---

Hm, it seems like this may be "as designed", as mentioned here:

The extension system is designed to be cooperative and so that a single extension cannot mute results of other extensions. We have no plans of changing that. However, if you think that the TypeScript/JavaScript extension behaves incorrect for a given scenario that please file a new, separate issue with steps to reproduce. Thanks

• how to override the default definition microsoft/vscode#76231 (comment)

However, maybe this falls into "behaves incorrect for a given scenario", and deserves a bug in TypeScript, similar to this change to the "Duplicate Definitions" issue here:

• Duplicate definitions returned for go to definition  microsoft/TypeScript#24861

@jrieken @mjbvz do you have any guidance here? It would be great to allow users to have the capability of opting out of these extra definition entries when a better one exists. This would enable one-click Go to Definition, which seems like the UX that is desirable for users. Ideally automatically by the extension, but it would also be acceptable to have the user manually opt out of specific definitions in some files (as long as they do not have to edit the global.d.ts file in their node_modules, which does not seem like a tenable solution).

We could also just create an issue in microsoft/vscode or microsoft/TypeScript to get some official feedback.

[karlhorky]

@iChenLei another idea: since you're using languages.registerDefinitionProvider here:

vscode-css-modules/src/extension.ts

Lines 23 to 26 in 4fc1c2a

	languages.registerDefinitionProvider(
	mode,
	new CSSModuleDefinitionProvider(options)
	)

...what do you think about trying to also declare the same declare module *.module.css/sass/scss types like in the Next.js global config? (eg. with any, as below) Maybe then your type would override this? (or maybe it doesn't work like I'm hoping?)

declare module '*.module.css' {
  const classes: any
  export default classes
}

declare module '*.module.sass' {
  const classes: any
  export default classes
}

declare module '*.module.scss' {
  const classes: any
  export default classes
}

[zardoy]

@karlhorky Only in case of TypeScript/JavaScript/Vue there is an API to take full control over definitions / completions and so on returned by TS language service with TS plugins. I've made an extension that removes generic definition: https://github.com/zardoy/typescript-vscode-plugins/ (enabled by default with miscDefinitionImprovement, source code).

I also noticed that defition priority is not consistent. With "editor.gotoLocation.multipleDefinitions": "gotoAndPeek" it was always revealing .css file definition first, but this week it started to focus on .d.ts file instead, I have no idea why, but because of this I made solution mentioned above.

[karlhorky]

I'm using the React CSS modules extension (repo) at the moment, which appears to avoid the problem somehow... 🤔

Edit: Actually sorry, this is incorrect - the issue is still present over there.

[karlhorky]

@zardoy I'm not sure what you mean by "TypeScript/JavaScript/Vue", but reading the source code of your extension TypeScript Essential Plugins (repo) it seems there may be code that could provide a way forward. Some interesting parts:

    proxy.getDefinitionAndBoundSpan = (fileName, position) => {
      const prior = info.languageService.getDefinitionAndBoundSpan(fileName, position)
      // ...
      prior.definitions = prior.definitions.filter(({ fileName, containerName, containerKind, kind, name, ...rest }) => {
        // ...
        if (moduleDeclaration?.name.text === '*.module.css') return false
        return true
      })
      // ...
    })

• https://github.com/zardoy/typescript-vscode-plugins/blob/a102a76786a0292aa8a9a46fb6cc43595e8871a7/typescript/src/definitions.ts#L7-L8
• https://github.com/zardoy/typescript-vscode-plugins/blob/a102a76786a0292aa8a9a46fb6cc43595e8871a7/typescript/src/definitions.ts#L123
• https://github.com/zardoy/typescript-vscode-plugins/blob/a102a76786a0292aa8a9a46fb6cc43595e8871a7/typescript/src/definitions.ts#L141

If I'm understanding correctly, this is filtering out previous definitions if they have the name *.module.css, which could also be extended to include *.module.scss and *.module.sass.

@iChenLei @clinyong what do you think about this approach?