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

No way to ensure an extension is prioritized as a rename provider in a language #115354

Open
atscott opened this issue Jan 29, 2021 · 4 comments
Open
Assignees
Labels
api rename under-discussion Issue is under discussion for relevance, priority, approach

Comments

@atscott
Copy link
Contributor

atscott commented Jan 29, 2021

We are implementing rename functionality and have found some guidance about how to ensure our extension "wins". This is based on the fact that rename takes the results from the first extension to return anything, the scoring implementation, and finally how the sorting handles when there is a score tie.

Unfortunately, the option for depending on the built-in extension doesn't work for us. The Angular Language Service provides features for both html and typescript languages. We've found that if the first document opened is an html file, the built in typescript extension does not register its providers, but our extension does. Then when a TS file is opened, the TS extension registers as a rename provider, meaning it will "win" the registration time tie breaker. The result is that renames will not work correctly from TS files.

We haven't found any additional guidance in existing issues or documentation on how to approach this. I guess I would categorize this as a feature request to make the provider registry for renaming more configurable.

@jrieken jrieken added api rename under-discussion Issue is under discussion for relevance, priority, approach labels Jan 29, 2021
@jrieken
Copy link
Member

jrieken commented Jan 29, 2021

Yes - there is no good way to achieve this. You continue the funky-path, e.g your extension could simply activate the TS/JS extension but that's all from "hacker land". I also don't know a good solution to this one. These the options

  • let the user choose - dynamically or via setting
  • add a setting to TS to disable its rename feature
  • NOT an option: allow extension A to mute extension B

@atscott
Copy link
Contributor Author

atscott commented Jan 29, 2021

Hi @jrieken, thanks for the quick response! I like the idea of letting the user choose which extension takes precedence, either dynamically or via a setting.

As for what we can do at the moment, while it feels very "hacker land", it's at least explicit and predictable behavior if it can be made to work. I'm still having a bit of trouble figuring that part out though.

In our extension, before we start our LanguageClient we try to activate the TS/JS extension with const ext = vscode.extensions.getExtension('vscode.typescript-language-features'); and await ext?.activate();. This doesn't seem to have the desired effect though.
If the first file opened was an HTML file, I still see the "Initializing TS/JS language features" notification the first time a TS file is open and renames from TS files are still handled by the built in extension. I would assume this means I wasn't able to get the TS/JS extension to register providers before ours (or prevent it from registering again when the first TS file is open).

We've also wondered investigating if re-registering ourselves as a rename provider after the first TS file is opened could do the trick but haven't tried it out yet.

atscott added a commit to atscott/vscode-ng-language-service that referenced this issue Feb 1, 2021
This commit adds rename capability to the Angular Language Service
extension. It's important to note that vscode only uses results from a
single rename provider. We cannot currently ensure that Angular takes
precedence over TypeScript when renaming from TS files
(see microsoft/vscode#115354) so this feature
is only available in _external_ template files (it is neither available for inline
templates nor other locations in TS files).
atscott added a commit to atscott/vscode-ng-language-service that referenced this issue Feb 1, 2021
This commit adds rename capability to the Angular Language Service
extension. It's important to note that vscode only uses results from a
single rename provider. We cannot currently ensure that Angular takes
precedence over TypeScript when renaming from TS files
(see microsoft/vscode#115354) so this feature
is only available in _external_ template files (it is neither available for inline
templates nor other locations in TS files).
atscott added a commit to atscott/vscode-ng-language-service that referenced this issue Feb 1, 2021
This commit adds rename capability to the Angular Language Service
extension. It's important to note that vscode only uses results from a
single rename provider. We cannot currently ensure that Angular takes
precedence over TypeScript when renaming from TS files
(see microsoft/vscode#115354) so this feature
is only available in _external_ template files (it is neither available for inline
templates nor other locations in TS files).
kyliau pushed a commit to angular/vscode-ng-language-service that referenced this issue Feb 3, 2021
This commit adds rename capability to the Angular Language Service
extension. It's important to note that vscode only uses results from a
single rename provider. We cannot currently ensure that Angular takes
precedence over TypeScript when renaming from TS files
(see microsoft/vscode#115354) so this feature
is only available in _external_ template files (it is neither available for inline
templates nor other locations in TS files).
atscott added a commit to atscott/vscode-ng-language-service that referenced this issue May 7, 2021
Previously we had decided to disable renaming in TypeScript files
because of a limitation in VSCode
(microsoft/vscode#115354). This meant that we
could give consistent expectations for Angular renames: they only work
in external templates.

This change updates that expectation by providing the ability to rename
from strings in TypeScript files. Because TypeScript would not
provide any rename results for most strings (see note below), we can be sure that
Angular will be asked to provide its rename results for these cases.

Additionally, this moves the logic for determining if we should provide
renames outside the server. The benefit of this is that potential
consumers of the @angular/language-server for other editors would be
able to provide renames if they do not have the limitation that vscode
does (mentioned above).

Note that TS _can_ actually provide renames for some string literals so
we still have to be more specific than allowing renames for all string
literals (ex: `type MyType = 'a'|'b'` - TSLS can rename the string union and
assignments to it).
If there were an assignment in a template to an value with the string
union type, we would not be guaranteed to pick this up if the rename were done
from a TypeScript file.
atscott added a commit to atscott/vscode-ng-language-service that referenced this issue May 7, 2021
Previously we had decided to disable renaming in TypeScript files
because of a limitation in VSCode
(microsoft/vscode#115354). This meant that we
could give consistent expectations for Angular renames: they only work
in external templates.

This change updates that expectation by providing the ability to rename
from strings in TypeScript files. Because TypeScript would not
provide any rename results for most strings (see note below), we can be sure that
Angular will be asked to provide its rename results for these cases.

Additionally, this moves the logic for determining if we should provide
renames outside the server. The benefit of this is that potential
consumers of the @angular/language-server for other editors would be
able to provide renames if they do not have the limitation that vscode
does (mentioned above).

Note that TS _can_ actually provide renames for some string literals so
we still have to be more specific than allowing renames for all string
literals (ex: `type MyType = 'a'|'b'` - TSLS can rename the string union and
assignments to it).
If there were an assignment in a template to an value with the string
union type, we would not be guaranteed to pick this up if the rename were done
from a TypeScript file.

fixes angular#1319
atscott added a commit to atscott/vscode-ng-language-service that referenced this issue May 7, 2021
Previously we had decided to disable renaming in TypeScript files
because of a limitation in VSCode
(microsoft/vscode#115354). This meant that we
could give consistent expectations for Angular renames: they only work
in external templates.

This change updates that expectation by providing the ability to rename
from strings in TypeScript files. Because TypeScript would not
provide any rename results for most strings (see note below), we can be sure that
Angular will be asked to provide its rename results for these cases.

Additionally, this moves the logic for determining if we should provide
renames outside the server. The benefit of this is that potential
consumers of the @angular/language-server for other editors would be
able to provide renames if they do not have the limitation that vscode
does (mentioned above).

Note that TS _can_ actually provide renames for some string literals so
we still have to be more specific than allowing renames for all string
literals (ex: `type MyType = 'a'|'b'` - TSLS can rename the string union and
assignments to it).
If there were an assignment in a template to an value with the string
union type, we would not be guaranteed to pick this up if the rename were done
from a TypeScript file.

fixes angular#1319
kyliau pushed a commit to angular/vscode-ng-language-service that referenced this issue May 13, 2021
Previously we had decided to disable renaming in TypeScript files
because of a limitation in VSCode
(microsoft/vscode#115354). This meant that we
could give consistent expectations for Angular renames: they only work
in external templates.

This change updates that expectation by providing the ability to rename
from strings in TypeScript files. Because TypeScript would not
provide any rename results for most strings (see note below), we can be sure that
Angular will be asked to provide its rename results for these cases.

Additionally, this moves the logic for determining if we should provide
renames outside the server. The benefit of this is that potential
consumers of the @angular/language-server for other editors would be
able to provide renames if they do not have the limitation that vscode
does (mentioned above).

Note that TS _can_ actually provide renames for some string literals so
we still have to be more specific than allowing renames for all string
literals (ex: `type MyType = 'a'|'b'` - TSLS can rename the string union and
assignments to it).
If there were an assignment in a template to an value with the string
union type, we would not be guaranteed to pick this up if the rename were done
from a TypeScript file.

fixes #1319
@ivanwonder
Copy link

I have a very hacker way to achieve this.
Assuming there are only two extensions, the TS/JS extension and angular extension, and the TS/JS extension win, If I make the TS/JS extension reject the rename request by writing a ts server plugin, the vscode will ask the next extension which will be the angular extension.
But if there is another extension, I think this cannot make sure the angular extension always wins.

ivanwonder added a commit to ivanwonder/angular that referenced this issue Jan 15, 2022
… Angular

When the user wants to rename a symbol in the ts file. the vscode will
ask the rename provider lists by the turn which is registered by the
extensions and order by the score. If the first extension returns the
result, the vscode will break the loop and apply the result, if the
first extension cannot rename the symbol, for example, the most string,
the built-in ts server cannot rename it(I think this is the call stack
[1][1]->[2][2]->[3][3]->[4][4]), the vscode will ask the second extension
in the list.

Because the built-in ts extension and Angular extension have the same
high score, if the built-in ts extension is the first(depends on the
time when to register it), the result will be provided by the built-in
extension, but we hope the Angular provide it, so this plugin will
delegate rename requests and reject the request for the built-in ts
server.

In the Angular LS, it only provides the rename info only when it is an
Angular project, if the first provider is the Angular LS and it is not
an Angular project, the built-in extension should provide the rename
info.

So this plugin will apply to the built-in TS/JS extension, and delegate
rename requests, it provides the rename info, only when it is an Angular
project, otherwise, it will return info by the default built-in ts server
rename provider.

See [here][5] for more info.

[1]: https://github.com/microsoft/TypeScript/blob/b2d2f085e249b5e1fdf375dd0c1b182b2c0b8c92/src/services/rename.ts#L11
[2]: https://github.com/microsoft/vscode/blob/94c9ea46838a9a619aeafb7e8afd1170c967bb55/extensions/typescript-language-features/src/languageFeatures/rename.ts#L41
[3]: https://github.com/microsoft/vscode/blob/ad5dd4e7360655563afd062b710276477409e648/src/vs/workbench/api/common/extHostLanguageFeatures.ts#L676
[4]: https://github.com/microsoft/vscode/blob/ea4d99921cc790d49194e897021faee02a1847f7/src/vs/editor/contrib/rename/rename.ts#L69
[5]: microsoft/vscode#115354
ivanwonder added a commit to ivanwonder/angular that referenced this issue Jan 17, 2022
… Angular

When the user wants to rename a symbol in the ts file. the vscode will
ask the rename provider lists by the turn which is registered by the
extensions and order by the score. If the first extension returns the
result, the vscode will break the loop and apply the result, if the
first extension cannot rename the symbol, for example, the most string,
the built-in ts server cannot rename it(I think this is the call stack
[1][1]->[2][2]->[3][3]->[4][4]), the vscode will ask the second extension
in the list.

Because the built-in ts extension and Angular extension have the same
high score, if the built-in ts extension is the first(depends on the
time when to register it), the result will be provided by the built-in
extension, but we hope the Angular provide it, so this plugin will
delegate rename requests and reject the request for the built-in ts
server.

In the Angular LS, it only provides the rename info only when it is an
Angular project, if the first provider is the Angular LS and it is not
an Angular project, the built-in extension should provide the rename
info.

So this plugin will apply to the built-in TS/JS extension, and delegate
rename requests, it provides the rename info, only when it is an Angular
project, otherwise, it will return info by the default built-in ts server
rename provider.

See [here][5] for more info.

[1]: https://github.com/microsoft/TypeScript/blob/b2d2f085e249b5e1fdf375dd0c1b182b2c0b8c92/src/services/rename.ts#L11
[2]: https://github.com/microsoft/vscode/blob/94c9ea46838a9a619aeafb7e8afd1170c967bb55/extensions/typescript-language-features/src/languageFeatures/rename.ts#L41
[3]: https://github.com/microsoft/vscode/blob/ad5dd4e7360655563afd062b710276477409e648/src/vs/workbench/api/common/extHostLanguageFeatures.ts#L676
[4]: https://github.com/microsoft/vscode/blob/ea4d99921cc790d49194e897021faee02a1847f7/src/vs/editor/contrib/rename/rename.ts#L69
[5]: microsoft/vscode#115354
ivanwonder added a commit to ivanwonder/angular that referenced this issue Jan 20, 2022
… Angular

When the user wants to rename a symbol in the ts file VSCode
will ask the rename providers for the answer in turn. If the
first extension returns the result, the VSCode will break the
loop and apply the result. If the first extension cannot rename
the symbol, VSCode will ask the second extension in the
list (built-in TS/JS extension, Angular LS extension, etc.).
In other words, VSCode takes the result from only one rename provider
and the order depends on registration timing, scoring.

Because the built-in ts extension and Angular extension have the
same high score, if the built-in ts extension is the
first(depends on the time the extension was registered), the result
will be provided by the built-in extension. We want Angular to
provide it, so this plugin will delegate rename requests and reject
the request for the built-in ts server.

The Angular LS only provides the rename info when working within
an Angular project. If we cannot locate Angular sources in the
project files, the built-in extension should provide the rename info.

This plugin will apply to the built-in TS/JS extension and delegate
rename requests to the Angular LS. It provides the rename info only
when it is an Angular project. Otherwise, it will return info by the
default built-in ts server rename provider.

See [here][1] for more info.

[1]: microsoft/vscode#115354
AndrewKushnir pushed a commit to angular/angular that referenced this issue Jan 21, 2022
… Angular (#44696)

When the user wants to rename a symbol in the ts file VSCode
will ask the rename providers for the answer in turn. If the
first extension returns the result, the VSCode will break the
loop and apply the result. If the first extension cannot rename
the symbol, VSCode will ask the second extension in the
list (built-in TS/JS extension, Angular LS extension, etc.).
In other words, VSCode takes the result from only one rename provider
and the order depends on registration timing, scoring.

Because the built-in ts extension and Angular extension have the
same high score, if the built-in ts extension is the
first(depends on the time the extension was registered), the result
will be provided by the built-in extension. We want Angular to
provide it, so this plugin will delegate rename requests and reject
the request for the built-in ts server.

The Angular LS only provides the rename info when working within
an Angular project. If we cannot locate Angular sources in the
project files, the built-in extension should provide the rename info.

This plugin will apply to the built-in TS/JS extension and delegate
rename requests to the Angular LS. It provides the rename info only
when it is an Angular project. Otherwise, it will return info by the
default built-in ts server rename provider.

See [here][1] for more info.

[1]: microsoft/vscode#115354

PR Close #44696
@MrAndersen1
Copy link

MrAndersen1 commented Mar 21, 2022

I ran into the same behavior when registering a rename provider for typescript. I would argue for having a solution where both rename providers (our own and typescript's) would be useable at the same time.

My situation is that when a user renames an 'x' then we have to rename a corresponding 'y' too or things will break later. In typescript this was neatly done by me finding 'y' when there was an attempt to rename an 'x', executing the vscode.executeDocumentRenameProvider for both 'x' and 'y' with their new names but not returning anything in my provider during this time so that I got results from typescripts own provider, merge the edits from these results and return the merge from my provider.

I learned today that this isn't exactly supported, hopefully it will be in the future because being able to reuse typescripts provider saves a lot of work.

josmar-crwdstffng pushed a commit to josmar-crwdstffng/angular that referenced this issue Apr 8, 2022
… Angular (angular#44696)

When the user wants to rename a symbol in the ts file VSCode
will ask the rename providers for the answer in turn. If the
first extension returns the result, the VSCode will break the
loop and apply the result. If the first extension cannot rename
the symbol, VSCode will ask the second extension in the
list (built-in TS/JS extension, Angular LS extension, etc.).
In other words, VSCode takes the result from only one rename provider
and the order depends on registration timing, scoring.

Because the built-in ts extension and Angular extension have the
same high score, if the built-in ts extension is the
first(depends on the time the extension was registered), the result
will be provided by the built-in extension. We want Angular to
provide it, so this plugin will delegate rename requests and reject
the request for the built-in ts server.

The Angular LS only provides the rename info when working within
an Angular project. If we cannot locate Angular sources in the
project files, the built-in extension should provide the rename info.

This plugin will apply to the built-in TS/JS extension and delegate
rename requests to the Angular LS. It provides the rename info only
when it is an Angular project. Otherwise, it will return info by the
default built-in ts server rename provider.

See [here][1] for more info.

[1]: microsoft/vscode#115354

PR Close angular#44696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api rename under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

4 participants