fix(lsp): support npm workspaces and fix some resolution issues #24627
Conversation
| "textDocument/hover", | ||
| json!({ | ||
|
|
||
| // project1 resolution |
dsherret
changed the title
| let discover_result = match scope.to_file_path() { | ||
| Ok(scope_dir_path) => { | ||
| let paths = [scope_dir_path]; | ||
| Workspace::discover( |
There was a problem hiding this comment.
The name Workspace is a little confusing because it actually contains information about where it started discovery from. Maybe we should rename it to WorkspaceContext or something like that? denoland/deno_config#93
There was a problem hiding this comment.
denoland/deno_config#94 -- We can also now re-org this to cache the workspace loading between discoveries.
dsherret
changed the title
|
I have an example of npm workspaces working without byonm, but failing with it in the lsp for some reason. This should be good enough for the first pass though. |
| @@ -11917,6 +11917,11 @@ fn lsp_node_modules_dir() { | |||
|
|
|||
| assert!(!temp_dir.path().join("node_modules").exists()); | |||
|
|
|||
| // a lockfile will be created here because someone did an explicit cache | |||
| let lockfile_path = temp_dir.path().join("deno.lock"); | |||
| assert!(lockfile_path.exists()); | |||
There was a problem hiding this comment.
Running deno cache creates a lockfile here so I think this is ok because it aligns with that now?
| }, | ||
| "newText": "''", | ||
| "newText": "'')", |
There was a problem hiding this comment.
Bug fix. Should be no semis here.
| cached_deno_config_fs: &(dyn DenoConfigFs + Sync), | ||
| deno_json_cache: &(dyn DenoJsonCache + Sync), | ||
| pkg_json_cache: &(dyn PackageJsonCache + Sync), |
There was a problem hiding this comment.
Sidenote: maybe we could create a wrapper like deno_unsync that would cheat the type system and not require these Sync trait bounds?
| @@ -11917,6 +11917,11 @@ fn lsp_node_modules_dir() { | |||
|
|
|||
| assert!(!temp_dir.path().join("node_modules").exists()); | |||
|
|
|||
| // a lockfile will be created here because someone did an explicit cache | |||
| let lockfile_path = temp_dir.path().join("deno.lock"); | |||
| assert!(lockfile_path.exists()); | |||
|
Workspace cache for deno_config is in denoland/deno_config#95 -- I will follow up and land that in a separate PR. |
Makes the lsp use the same code as the rest of the cli.
Makes the lsp use the same code as the rest of the cli.
Closes #24598
Closes #24505