Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: goToDefinition support default classes #260

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qyzzzz
Copy link
Contributor

@qyzzzz qyzzzz commented Feb 27, 2024

It seems that goToDefinition does not support jumping to the definition of import s from "a.scss'; s['xxx']. This is because the variable declaration is only added to appropriate dts lines in dtsLines.
To resolve this issue, the proposed solution in this PR is to modify declare property by using an interface to define the property. This will allow nameExports and property to be on the same line, enabling goToDefinition to jump to the correct line number. Of course, invalid nameExports will still be filtered out.

@astrodelete
Copy link

Hello,
Is this by any chance related to this issue we encountered in Webstorm?

@mrmckeb
Copy link
Owner

mrmckeb commented Mar 21, 2024

Hi @qyzzzz, this looks like a very interesting change. This also looks like a very big change and unfortunately I'm away at the moment, but will try to look in a week or so when I'm back.

Can you confirm that you've tested that everything still works for users with other configurations than yours? It's unfortunately quite hard to test, you need to do it manually.

@mrmckeb mrmckeb self-assigned this Mar 21, 2024
@qyzzzz
Copy link
Contributor Author

qyzzzz commented Mar 26, 2024

Hi @mrmckeb, sorry for delay here.
I have determined the feasibility of this solution by reviewing and comparing the unit test snapshots in your project.
Additionally, I have created a separate project for manual effect testing. In this round of modifications, I primarily examined the impact of these changes on the goToDefinition, namedExports, and allowUnknownClassnames options.
https://github.com/qyzzzz/typescript-plugin-css-modules-goToDefinition-reproduction
Unfortunately, I am currently using VSCode and have not tested with Webstorm.😅

@mrmckeb mrmckeb added this to the v6.0.0 milestone Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants