Skip to content

fix: install typescript for the language server#76

Closed
jasonlyu123 wants to merge 4 commits into
zed-extensions:mainfrom
jasonlyu123:install-ts
Closed

fix: install typescript for the language server#76
jasonlyu123 wants to merge 4 commits into
zed-extensions:mainfrom
jasonlyu123:install-ts

Conversation

@jasonlyu123

@jasonlyu123 jasonlyu123 commented May 4, 2026

Copy link
Copy Markdown

Svelte language server plans to move TypeScript to peerDependency as part of TypeScript 6 support. The reason is to allow people to continue using TypeScript 5 if needed. This means npm won't automatically install TypeScript anymore, so the extension needs to install it manually. But it's still beneficial to have this to ensure TypeScript is updated.

@cla-bot

cla-bot Bot commented May 4, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @jasonlyu123 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@jasonlyu123

Copy link
Copy Markdown
Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label May 4, 2026
@cla-bot

cla-bot Bot commented May 4, 2026

Copy link
Copy Markdown

The cla-bot has been summoned, and re-checked this pull request!

@jasonlyu123

Copy link
Copy Markdown
Author

I just noticed that npm actually automatically installs peerDependency in npm 7+. So it's not that important. But this still has the benefit of ensuring that TypeScript is updated.

@ghostdevv ghostdevv changed the title Install Typescript for the language server fix: install typescript for the language server Jun 1, 2026
@ghostdevv ghostdevv added minor Used to trigger a minor version bump with Zed Zippy and removed minor Used to trigger a minor version bump with Zed Zippy labels Jun 1, 2026
@ghostdevv

Copy link
Copy Markdown
Collaborator

The current Zed extension API checks the node_modules dir to tell if a package is installed, which will find TypeScript since it's a peer dep but it's not really the intended effect as it's not added to the root package.json like the svelte language server and ts plugin are. We could change the install order, but it wouldn't solve it for people who aren't using the extension for the first time.

https://github.com/zed-industries/zed/blob/b4ce020d832e95405fb7cc948730a2ef364d1be5/crates/node_runtime/src/node_runtime.rs#L944-L948

There are a few workarounds I can think of, but do you think it's worth it? @jasonlyu123

@jasonlyu123

Copy link
Copy Markdown
Author

Not sure I follow what the difference is from your intended effect. Are you saying the "check for update" part won't work? Isn't the function you linked checking the target module's package.json?

@ghostdevv

Copy link
Copy Markdown
Collaborator

Not sure I follow what the difference is from your intended effect. Are you saying the "check for update" part won't work? Isn't the function you linked checking the target module's package.json?

The npm install commands run in ~/.local/share/zed/extensions/work/svelte for me (where npm creates a package.json with just the dependencies in it), however the fn that checks if a package is installed just looks for ~/.local/share/zed/extensions/work/svelte/node_modules/<module-name>/package.json. Since npm's node_modules is flat, it finds typescript as it was auto installed as a peer dep, and as such we don't explicitly install it at the top level, i.e. it's not in our dependencies of that top level generated package.json.

@jasonlyu123

Copy link
Copy Markdown
Author

I did intend to install it at the top level. Originally, I just didn't realise newer versions of npm actually auto-install peer. But keep the PR since it can also "update" the typescript version.

Now that TypeScript 7 stable is on the horizon, auto-installing the latest TypeScript version will instead break the language server since we still need TypeScript 6. Therefore closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants