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

Inconsistent LSP: Rename behaviour #192

Open
martinstark opened this issue Jan 10, 2023 · 9 comments
Open

Inconsistent LSP: Rename behaviour #192

martinstark opened this issue Jan 10, 2023 · 9 comments

Comments

@martinstark
Copy link

Describe the bug

I ran into an issue where if I run LSP: Rename once on a property it detects 10 occurrences across 8 files. I then run LSP: Rename again on the same (just renamed) property and it only detects 2 occurrences across 2 files.

I'm in a monorepo project structured like so:

// rootFolder is set up as sublime project root

rootFolder/package1/src // source files
rootFolder/package1/dist // locally compiled package
rootFolder/package2/src // depends on package1
rootFolder/package2/dist
rootFolder/package3/src // depends on package1 and 2
rootFolder/package3/dist
// etc

The second run of LSP: Rename seems to only find occurrences within the package path that the property to be renamed originates from, e.g. package1.

I understand that the monorepo structure with adjacent packages consuming the locally built /dist folders requires restarts of the LSP server after re-building the project. WebStorm handles this without issues when performing refactor/renaming, but I'm trying to migrate to Sublime.

For a couple of rounds it consistently worked, by making sure that I:

  1. do LSP: Rename (detect 10 occurrences across 8 files)
  2. rebuild project to make new type definitions available to adjacent packages
  3. restart LSP TypeScript server to pick up file changes
  4. do LSP: Rename again to "revert" the changes (detect 10/8)
  5. rebuild project
  6. restart LSP TypeScript server
  7. ...

After a couple of runs it starts detecting only 2 occurrences, and I can't get it to detect all 10/8 by rebuilding project, restarting LSP server, or restarting Sublime.

If I open a project file that references the property I'm trying to edit, then go back to edit the type definition via LSP: Rename, it will now detect 3 occurrences across 3 files. If I keep opening the files I know reference the property LSP: Rename will eventually detect all of them again. If I trigger LSP: Rename in a different file referencing the same property, it will sometimes go back to detecting all occurrences, and sometimes detect its own subset of occurrences within its package.

To Reproduce

Steps to reproduce the behaviour:

  1. Open a TypeScript monorepo as a project in Sublime
  2. Build and install dependencies
  3. Perform a LSP: Rename on a property that is used across multiple package folders
  4. See it correctly find all references
  5. Save All
  6. Re-build the project
  7. Restart LSP-Typescript server
  8. Perform a LSP: Rename on the same property (that now has a new name)
  9. ... repeat steps 3 through 9 some number of times
  10. See LSP: Rename fail to correctly find all references

Expected behavior
LSP: Rename should find all references to a property every time it is run.

Screenshots
n/a

Environment (please complete the following information):

  • OS: Arch Linux
  • Sublime Text version: 4143
  • LSP version: 1.21.0
  • Language servers used: LSP-Typescript, LSP-Eslint

Additional context
n/a

@rchl
Copy link
Member

rchl commented Jan 10, 2023

It would be good if you could find a public project that reproduces this issue as otherwise there is not much I can do to even investigate it. It seems to work fine on a non-monorepo, no-build-required project at least.

If you have a chance then you could also check with VSCode since if the bug reproduces there too, that would likely mean that the issue is in TypeScript.

@martinstark
Copy link
Author

Thank you for the quick response, I'll see if I can set up a minimal sample repo where I can consistently reproduce it.

(off topic: is there a place where I can ask general questions about LSP? It would be nice if there was a "Question"-type issue option here on github)

@rchl
Copy link
Member

rchl commented Jan 11, 2023

There is Discord channel that is quite popular and functional: https://github.com/sublimelsp/LSP#getting-help

We could also enable Discussions if it makes sense.

@martinstark
Copy link
Author

Closing as I ended up moving away from sublime in the end

@predragnikolic
Copy link
Member

predragnikolic commented Jan 12, 2023

For people running in this issue, try installing https://packagecontrol.io/packages/LSP-file-watcher-chokidar , and see if that fixes your issue.

If the issue is still there, try providing us a repository that can reproduce the issue, as that would be helpful for pinpointing the cause.


EDIT:What I wrote will probably not help, valid because because of the comment bellow,
and this

As for file watching, the typescript LSP server doesn't support external file watcher so none of this will make any difference. In that case the server runs its own watcher. You can configure some aspects of it through tsconfig.json but I'm not sure you really should be touching it. I doubt that will fix any issues you have. :)

@martinstark
Copy link
Author

I was using LSP-typescript, and apparently it has its own file watcher, see: #188

@predragnikolic
Copy link
Member

When I have time I will try to repro this.

@martinstark
Copy link
Author

martinstark commented Jan 12, 2023

I'm unsure if the repo has to be of a certain size for the issue to start appearing, my plan was to set up a minimal mono-repo using yarn to automatically symlink local workspace dependencies to each other, using the structure outlined in the first post. Requires setting up package.json's and copying relevant files to the /dist folder on transpile.

// rootFolder/package1/src/index.ts -> transpiles to rootFolder/package1/dist

type SomeObjectTypeDefinition {
  prop1: string; // occurrence 1
}

export const someObject: SomeObjectTypeDefinition = {
  prop1: "value"  // occurrence 2
}

// rootFolder/package2/src/index.ts -> transpiles to rootFolder/package2/dist 

import { someObject } from "package1";

const prop1 = someObject.prop1;  // occurrence 3

// rootFolder/package3/src/index.ts

import { someObject } from "package1";

const { prop1 } = someObject;  // occurrence 4

Renaming prop1 should in theory find 4 occurrences across 3 files. Saving, re-transpiling and restarting the LSP server should then again find 4/3. And so on.

The TypeScript LSP is generally able to resolve Renames that crosses package borders as long as adjacent packages are transpiled to their repsective /dist folders and the LSP server is restarted if the files are deleted and re-transpiled.

I would expect the same result could be achieved by importing multiple separate git repositories into a single Sublime project, and linking them manually via yarn link.

@rchl
Copy link
Member

rchl commented Jan 28, 2023

I'm transferring this to LSP-typescript since it's unlikely to be an issue with how rename is implemented but rather with the server response in specific situations.

@rchl rchl transferred this issue from sublimelsp/LSP Jan 28, 2023
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

No branches or pull requests

3 participants