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

(fix) store rework #1531

Merged
merged 19 commits into from Jun 21, 2022
Merged

(fix) store rework #1531

merged 19 commits into from Jun 21, 2022

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Jun 19, 2022

Reworks $store handling in the code base. Previously, svelte2tsx would transform $store access to something like (__get(store), $store). This is now no longer the case. This means we need to add some extra code to some of the intellisense features:

  • if someone renames a $store, store also needs to be renamed and vice versa
  • if someone wants to find references of a $store, we also need to find store references and vice versa
  • if someone wants to go to the definition of a $store, we need to reroute that to store
  • if someone wants to go to the implementation of a $store, we need to reroute that to store
  • diagnostics of wrong $store usages need to be handled

This means some more code overall, but I feel it's the better choice since we had so many edge cases with our transformation other time, and #1475 being an upstream bug we cannot solve ourselves but need to solve for the new transformation.

@jasonlyu123
Copy link
Member

jasonlyu123 commented Jun 20, 2022

Maybe we also need to check the $store variable in the go-to-implementation. Probably only need to check for store for $store and don't need the other way around.

@dummdidumm
Copy link
Member Author

Do you mean "right-click on $store -> Go to implementations -> get taken to store"?

@dummdidumm
Copy link
Member Author

Added that, thanks for noticing. Does this look good to otherwise or is there something that needs changing (codestyle, missing cases)?

@dummdidumm dummdidumm merged commit e91eeba into sveltejs:master Jun 21, 2022
@dummdidumm dummdidumm deleted the store-rework branch June 21, 2022 07:48
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.

None yet

2 participants