Skip to content

nodePackages.typescript-language-server: add typescript dependency#73119

Closed
kira-bruneau wants to merge 1 commit intoNixOS:masterfrom
kira-bruneau:typescript-language-server
Closed

nodePackages.typescript-language-server: add typescript dependency#73119
kira-bruneau wants to merge 1 commit intoNixOS:masterfrom
kira-bruneau:typescript-language-server

Conversation

@kira-bruneau
Copy link
Contributor

Motivation for this change

typescript-language-server is a wrapper around tsserver and is useless without it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @malob

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Nov 9, 2019
@malob
Copy link
Member

malob commented Nov 29, 2019

This looks good to me! (Works on my macOS machine.)

Copy link
Member

@jD91mZM2 jD91mZM2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stumbled upon this after wondering why typescript-language-server didn't work like it should. Adding typescript as an installed package worked too - although this is obviously a much better solution.

@kira-bruneau kira-bruneau force-pushed the typescript-language-server branch from b6eb9c8 to 1b5ebc9 Compare May 22, 2020 02:31
@prusnak
Copy link
Member

prusnak commented May 30, 2020

Please rework your PR. It now has a merge conflict after PR #89184 has been merged

@prusnak prusnak added 2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment labels May 30, 2020
typescript-language-server is a wrapper around tsserver and is useless
without it
@kira-bruneau kira-bruneau force-pushed the typescript-language-server branch from 1b5ebc9 to d845152 Compare May 30, 2020 17:21
@kira-bruneau
Copy link
Contributor Author

@prusnak I just force pushed a change to bring it up-to-date.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 30, 2020
@felschr
Copy link
Member

felschr commented Sep 25, 2020

Just wondering would a local tsserver installation still override the default one with these changes?

@kira-bruneau
Copy link
Contributor Author

kira-bruneau commented Sep 25, 2020

@felschr No, right now it wouldn't. But I could update PATH using --suffix instead of --prefix so it would.

@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jan 2, 2021

PR got combined with a few other node updates. Closing so that the update goes trough smoothly.

@kira-bruneau kira-bruneau deleted the typescript-language-server branch January 4, 2021 05:07
jlesquembre added a commit to jlesquembre/nixpkgs that referenced this pull request Jan 20, 2022
…back

Make possible to use a different typescript version.
If there is already a typescript binary on your PATH, probably you want
to use that. This way it's possible to use a different typescript
version (with nix shell, direnv, npm, ...), but still have a fallback
version, so the lsp server doesn't fail to start

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

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants