"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 :)