use node kind checking more strict variant

[zardoy]

Hi my friend was using your react-ts extension and pointed to changelog, I was wondering why fileName filtering is done, which:

1. forces user to handle configuration with pain
2. filters out some potentially good definitions (bad)

@karlhorky it fixes Viijay-Kr/react-ts-css#68 (comment)

In aforementioned issue body you already provided code in Potential approach and I'm just using it here

[Viijay-Kr]

@zardoy thanks for the PR . I really appreciate.

I would have went with this approach with later versions as the first version of the plugin is to see how it works out.

Nonetheless thanks for your PR. I will do a round of testing and merge this one .

👍

However Viijay-Kr/react-ts-css#68 (comment)
is unrelated to this

[zardoy]

However Viijay-Kr/react-ts-css#68 (comment) is unrelated to this

I tested provided implementation against repro provided in this comment. Also modules setting can be removed in ext if your okay with this implementation.

I would have went with this approach with later versions as the first version of the plugin is to see how it works out.

Probably it wasn't working because fileName is absolute path but modules had relative paths. You needed to use languageService.getProgram()!.getCurrentDirectory() or other methods to make them absolute.

[Viijay-Kr]

@zardoy

However Viijay-Kr/react-ts-css#68 (comment) is unrelated to this

I tested provided implementation against repro provided in this comment. Also modules setting can be removed in ext if your okay with this implementation.

I would have went with this approach with later versions as the first version of the plugin is to see how it works out.

Probably it wasn't working because fileName is absolute path but modules had relative paths. You needed to use languageService.getProgram()!.getCurrentDirectory() or other methods to make them absolute.

Viijay-Kr/react-ts-css#68

The problem in that issue is that the plugin doesn't work sometimes.

Its actually because the plugin is not able to be loaded since I was not shipping my node_modules as part of the extension

I have a version shipped with node_modules

[Viijay-Kr]

@zardoy Viijay-Kr/react-ts-css#68 (comment)

This should make the plugin work.

Regarding this improvement PR
I like this PR. I will merge this tomorrow.

[Viijay-Kr]

So far , this method filter out results from nextjs declaration and react-scripts declaration files. which has a clear declaration type ancestor

However for vite based apps , the definition result points to this location in the declaration file.
So this specific node in the AST doesn't have any Module Declaration ancestor.

Therefore

if (ts.isModuleDeclaration(node)) {

}

this block never gets executed.

Do you have any other ideas to solve this ?

[zardoy]

Interesting, didn't know of these cases, and have no ideas for clean solution.

Do you have any other ideas to solve this ?

Maybe we can include some additional pattern checks as in case of Vite these names are straightforwad:

  if (kind === ts.ScriptElementKind.indexSignatureElement && name === '__index') {
              if (containerName === 'CSSModule' || containerName === 'CSSModuleClasses') return false
              // ... findNodeAtPosition

[Viijay-Kr]

Interesting, didn't know of these cases, and have no ideas for clean solution.
In theory a library author could emit their declarations however they want.

Maybe we can include some additional pattern checks as in case of Vite these names are straightforwad:

Can try this out