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.

Sign up for GitHub

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

Jump to bottom

Support rename in Svelte files #110

Closed
dummdidumm opened this issue May 23, 2020 · 5 comments
Closed

Support rename in Svelte files #110

dummdidumm opened this issue May 23, 2020 · 5 comments
Labels
feature request New feature or request

Comments

@dummdidumm
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently, I cannot do rename refactorings in svelte files.

Describe the solution you'd like
Allow rename refactorings inside svelte files to work so that

  • I can change prop names and all dependent files get updated
  • I can change variables/functions inside svelte files
  • I can change variables/functions outside in js/ts files and if they are used inside svelte files, those are updated aswell

Additional context
I started prototyping this. Renames of variables/functions inside svelte files can be implemented quickly.

However implementing prop renames requires changes to svelte2tsx. It seems that right now there is no mapping from export let X to the generated jsx-prop.

Another problem is that when you do a rename inside ts/js, the language server does not pick that up because he is only registered to react to changes of svelte files. To fix that, it would need to register for js/ts files, too, which requires some work to differ between svelte and ts/js documents and not for example run diagnostics of the HTMLPlugin on js/ts files. Maybe this could come as a second step, after renaming inside svelte files works.
@dummdidumm dummdidumm added the feature request New feature or request label May 23, 2020
This was referenced May 23, 2020
Closed
Open
dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Jun 6, 2020
(feat) basic rename
675d4e1 
sveltejs#110
@dummdidumm dummdidumm mentioned this issue Jun 6, 2020
Merged
4 tasks
dummdidumm added a commit that referenced this issue Jun 18, 2020
@dummdidumm
(feat) basic rename (#172)
f483271 
* (feat) basic rename

#110

* (feat) handle prop rename of component A inside component B

* (feat) support for rename of prop of A in A

* (fix) linting

* (fix) fix range conversion, fix word retrival, tests

* (fix) don't rename everything

* (feat) add prepareRename support

To show a message early that rename is not allowed in certain locations

* (fix) use modified mapRangeToOriginal

* (fix) don't rename in strings

* (fix) don't allow rename of import path

We cannot handle it at the moment

* (fix) support prop rename without types

For that svelte2tsx needs to write out the props as {prop: prop}

* (fix) prevent renames in error state

* (fix) use ts service for getting variable at position

* (fix) convert bind so that source mapping is correct
@dummdidumm
Copy link
Member Author

Rename so far now works for:

  • Variables inside a svelte file's script tag
  • Component props, but they can only be renamed inside the component which exports the prop

It does not yet work for:

  • Component props if you want to rename the prop but are in another file which uses that component
  • Generally attributes/bindings inside the template cannot be renamed when you want to trigger the rename in said template. Works inside script tag though.

@jasonlyu123 jasonlyu123 mentioned this issue Nov 10, 2020
Closed
@imekachi
Copy link

There is another scenario, where a svelte file imports a variable from another file (.js, .ts)
and you rename that variable in other file, it doesn't update variable imported in .svelte

I think it has something to do with referencing variables, because it cannot find reference from .svelte file as well
image

@dummdidumm
Copy link
Member Author

To support this we need to write a TypeScript plugin, which is tracked in #580

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue May 3, 2021
(feat) TypeScript plugin
c947c7e 
Initially support
- rename (doesn't work properly for all kinds of renames yet; need to filter out references inside generated code)
- diagnostics
- find references (need to filter out references inside generated code)

This makes all files TSX hardcoded for now, it seems the TS server is okay with importing tsx into js

sveltejs#580
sveltejs#550
sveltejs#342
sveltejs#110
@dummdidumm dummdidumm mentioned this issue May 3, 2021
Merged
dummdidumm added a commit that referenced this issue May 4, 2021
@dummdidumm
(feat) TypeScript plugin (#978)
505d7dc 
Initially support
- rename (doesn't work for prop renames yet)
- diagnostics
- find references
- go to definition
- go to implementation(s)

This makes all files TSX hardcoded for now, it seems the TS language server is okay with importing tsx into js

#580
#550
#342
#110
@dummdidumm
Copy link
Member Author

There exists a TypeScript plugin now which comes packaged with the VS Code extension and which you need to enable through the settings. It also is available standalone as a npm package if you need to use it outside of VS Code.
The plugin fixes the issue of renaming stuff inside TS/JS files and updating Svelte files, too.

@imekachi
Copy link

imekachi commented May 5, 2021 •
edited
Loading

I can confirm that everything works now in VS Code with the plugin and svelte.enable-ts-plugin: true

  • renaming from other file types
  • finding references from other file types

Thank you @dummdidumm

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Jun 3, 2021
(feat) rename of component props in other component
0755956 
Allow rename of component's A props in another component B.
sveltejs#110
@dummdidumm dummdidumm mentioned this issue Jun 3, 2021
Merged
dummdidumm added a commit that referenced this issue Jun 3, 2021
@dummdidumm
(feat) rename of component props in other component (#1043)
67f8a4a 
Allow rename of component's A props in another component B.
#110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@imekachi @dummdidumm