"Go to definition" does not work right

[dko-slapdash]

Describe the bug

Thanks for the awesome plugin!

I noticed that "Go to definition" feature doesn't jump to the CSS class though.

To Reproduce

Here is an animated GIF which shows the problem:

• OS: MacOS
• Version: current

[dko-slapdash]

...what I also want to say: your plugin is the only really working solution in the universe. I just spent 2 days only on bridging css-modules + typescript strong typing + babel. And it finally works (except the two issues I created here). It's a big responsibility - and a big respect - to support this solution, because (as I believe) the general success of css-modules (vs. other techniques like styled-components may significantly depend on the success of this plugin (if we consider that typescript will continue its dominance). E.g. the popular create-react-app tool does not currently support css-modules properly (they miss this particular plugin in their config). If the plugin appeared polished enough to be included in create-react-app, it would be a HUGE success.

[mrmckeb]

Hi @dko-slapdash, I'm actually on the Create React App team - the reason we haven't included this is because plugins for TypeScript only work in an IDE... which means you don't get errors during build, or on your CI. I hope the TS team addresses this in the near future.

As for this issue, it's definitely a known issue and I should have documented it.

The plugin works by loading CSS, processing it (Sass/Less/PostCSS), parsing it, and then we get an object back with keys representing the classes found.

To make this feature work, I would need to know the location of each class. I think we'd need to do a string search... which in some cases would be impossible, especially if class names are generated (as they can be in Less and Sass)... So it is possible, but not in all circumstances.

We may also be able to use sourcemaps for this, but again, it could be a fair amount of work.

If you have any thoughts or ideas though, I'd love to hear them.

And thanks for all of your feedback!

[dko-slapdash]

Oh wow, I didn’t realize you’re in create-react-app!

I think that even a raw string search by /^\.key\b/ where key is the property visible from TS side would be waaay better than nothing.

(BTW regarding less, I had to switch back from less to css unfortunately, because the plugin was stopping working silently; I can’t reproduce it stable enough though; btw, how do you debug the code? Is there a quick way to turn on some verbose logging in vscode in regards to this plugin?)

[mrmckeb]

There's a PR in to resolve the issue around silent failures - it was an oversight on my part.

I'm currently away on a business trip and I don't have a lot of time, but I hope to get that in in the next few days.

I think searching by key is fine, but the concern is that it won't catch all classnames... but I'm wondering if we could do something with sourcemaps instead... :)

[mrmckeb]

hi @dko-slapdash, just dropping a note to say that I haven't forgotten about this.

I think it's going to require using sourcemaps. Perhaps using something like this https://github.com/mozilla/source-map, or something more lightweight if possible.

[EECOLOR]

@mrmckeb Source maps won't help here as the icss stack generally doesn't supply source information when they are doing the transforms.

All exported elements are extracted from the :export pseudo class. Which might be generated by :local, @value or postcss messages that adhere to the 'icss spec'.

The ideal solution would be to include source information through the whole icss world, but that is not realistic. Next best thing is a 'simple' string search with something like a word boundary. It will not be perfect in all situations, for example:

.test {
  ...
}

.other > .test {
  ...
} 

Here the test export is created by both occurrences. And I don't think (I might be wrong) typescript has support for things originating in two locations.

I however think no-one will blame you for linking to the first occurrence.

If you later wanted to add some form of preview I could help you with the postcss plugin that helps you find the rule, :export prop or @value definition that is referred to.

For now, the string search option seems a safe, non-expensive way to make the plugin much more valuable.

[mrmckeb]

@EECOLOR, I complete agree - the only concern I have is that this won't work for many cases.

As you mentioned, it won't work for multiple occurences, it also won't work for generated/concatenated classnames, etc.

My thought was that we could possibly deal with the latter issue by creating a sourcemap before we extract the classnames... or as a separate process. As you mentioned, that's not going to be cheap.

If someone wants to put together a basic string-match in the meantime, I'd be happy to merge that in :)

[dko-slapdash]

BTW, VSCode supports multiple occurrences in the UI - i.e. if we create the following:

// a.ts
interface A {
  field1: number;
}

// b.ts
interface A {
  field2: string;
}

then it will be an interface A with 2 fields, and when doing "Go to definition", it will show 2 files and 2 locations for A.

P.S.
I'd say that the absence of "go to definition" doesn't hurt much presently. What really hurts is that the plugin doesn't work stable enough still: e.g. if I add a new classname to a css file, in 70% of cases VSCode doesn't recognize a reference to it in *.tsx file, and only vscode restart helps. I suspect it's still the same issue which was hot-patched in #41 - it used to crash tsserver, and after the patch, it doesn't crash, but just ignores all further changes in css file.

[ftzi]

Hey there! Very nice plugin, thank you very much for the work. here and on CRA. I don't think it would be bad to be included in the create-react-app even if it doesn't throw errors in compilation. Would already be a giant help :)

In VsCode, we can press F2 to rename a symbol. With this plugin, it doesn't work to rename both css and the .ts. Looks like it is the same problem as the OP pointed.

I still don't know how plugins works, but wouldn't a Regex fix it?

Edit: Also, won't open an issue for this one, because I don't know if your plugin could do it:
It is possible to autocomplete the path, when importing the styles file?

[mrmckeb]

HI @SrBrahma,

You're right, that is the same issue. It's something I'd like to fix - and I'd welcome a PR from anyone that has the time. It's just that I haven't personally had time to investigate the best way to achieve this.

Also, won't open an issue for this one, because I don't know if your plugin could do it:
It is possible to autocomplete the path, when importing the styles file?
I don't think it's possible right now - as the plugin needs the file to be imported before it picks it up. We could look to change that in future, but it would be a bit of work.

[AleksandrHovhannisyan]

I know this isn't the right place to say it, but I'd rather not pollute the repo with a pointless issue.

Thank you SO much for this plugin. As @dko-slapdash noted, this is literally the only thing that currently works. Great stuff.

[mrmckeb]

Thanks for the feedback @AleksandrHovhannisyan!

[michael-swarm]

I just wanted to add a note here that I managed to get this working as expected in VS Code by installing the CSS Modules extension. However, this causes there to be duplicate autocomplete options and duplicate definitions, which isn't ideal either.

[mrmckeb]

@michael-swarm if you're using that extension, I'd recommend not using this plugin. Both are doing the same thing, as you mentioned.

This is something that's fixable... but not for all cases. It's mostly that I haven't had the time to spend on it sorry - but we always welcome PRs.

[sosoba]

It's something I'd like to fix - and I'd welcome a PR from anyone that has the time. It's just that I haven't personally had time to investigate the best way to achieve this.

Could someone suggest which method of Typescript or LanguageHosts should be ovverided to do mapping the css file line to the .ts line?

[mrmckeb]

I'm not 100% sure on that either @sosoba, as I haven't attempted it yet. I assume we'll be able to use sourcemaps here to get the original lines/locations, I'm just not sure how to surface that in the IDE.

[NoamNol]

I did npx create-next-app@latest --ts and I can't go to definition. instead, vscode brings me to global.d.ts.
I know this is a different issue but it's related to the discussion here.

[lucasbasquerotto]

@NoamNol If you created a new project, your case may be that that it's using the Editor/IDE Typescript, instead of the typescript in your workspace.

The errors are shown normally (if you write a wrong classname, for example)?

If you are using VSCode, make sure you follow the steps defined at https://code.visualstudio.com/docs/typescript/typescript-compiling#_using-the-workspace-version-of-typescript

[NoamNol]

@lucasbasquerotto I configured it to use the workspace TypeScript but it doesn't matter.
No error for wrong classname.
Latest version of VSCode.
TypeScript 4.5.4.

[lucasbasquerotto]

@NoamNol It should give an error, so it seems the plugin is not being used.

One reason for that is that it's not using the typescript of your workspace, like I said previously. Another reason is that the typescript tsconfig.json file doesn't have:

plugins": [{ "name": "typescript-plugin-css-modules" }]

Can you verify that your tsconfig.json file has the plugin above, and that your package.json also has it in dependencies or devDependencies (and it's installed in node_modules)?

If it has, then I don't know what it could be, maybe someone else can help, but first check what I said above.

[NoamNol]

@lucasbasquerotto Thanks it works!
I thought npx create-next-app@latest --ts will do all the configuration but I was wrong

[FunctionDJ]

@NoamNol can you share more about your setup?
i've tried installing this plugin but it looks like the Next.js types still overwrite the plugin's types (i get taken to global.d.ts).
i have this in my tsconfig:

    "plugins": [
      {
        "name": "typescript-plugin-css-modules"
      }
    ]

[mrmckeb]

Hi @FunctionDJ, the Next.js types will be used during build, but these types will be used in VSCode (or other editors). Just make sure you've followed the instructions in the README :)

[max-mykhailenko]

My VSCode opens d.ts file with definition, how to open at least correct CSS file?

[piro0919]

@max-mykhailenko

I've been having the same problem for a long time, the problem went away when I used react-ts-css .
I would love for you to try it. 👍

[max-mykhailenko]

@piro0919 @mrmckeb thank you, works better now, now I have two definitions. How to tell VSCode to ignore d.ts files for css?

[piro0919]

@max-mykhailenko

I don't know the details, but what if you put the following settings in vscode?

  "editor.gotoLocation.multipleDefinitions": "goto",
  "editor.gotoLocation.multipleTypeDefinitions": "goto",

[OliverJAsh]

If I understand correctly this only works for Sass files at the moment. What work is left to do to make this work for regular CSS files? (I tried it but it didn't seem to work properly.)

Update: extracted to #175

Screen.Recording.2022-11-27.at.11.24.36.mov