search

microsoft / TypeScript-Sublime-Plugin

IO wrapper around TypeScript language services, allowing for easy consumption by editor plugins
Apache License 2.0
1.72k stars 237 forks source link

#538 Automatically check if node_modules/typescript/lib exists #720

Open tayelno opened 5 years ago

tayelno commented 5 years ago

Fixes #538

kylebebak commented 5 years ago

This would be a great improvement.

orta commented 4 years ago

Yeah, I agree - I'd like to get this in.

I'm a tad apprehensive about it being prioritized over the bundled version. vscode, vs and flow-for-vscode for example all have an option which lets you choose to have this version be prioritized.

Any chance we can have this as a setting somehow? The gaol being specifically to make people in to a project's arbitrary code being executed when they open the editor.

sahilrajput03 commented 3 years ago

Make this feasible please!

sahilrajput03 commented 3 years ago

After manually cloning my typescript package in packages folder after deleting it completely made it working seamless now!! react jsx works all good for me as on this date, happy to have this plugin thanks team!!

image

Yikes!!!!!!!!!!!!

tayelno commented 3 years ago

almost forgot about this, problem is that I have little to no knowledge on how to do this, but just want to confirm, only issue atm is that it needs to be added as a configurable option and not as a default behavior right ?

orta commented 3 years ago

Yeah, allow this to be opt-in and it's reasonable to me 👍🏻

sahilrajput03 commented 3 years ago

almost forgot about this, problem is that I have little to no knowledge on how to do this, but just want to confirm, only issue atm is that it needs to be added as a configurable option and not as a default behavior right ?

It juat works like that way, like simply cloning the way its mentioned in docs..., and no need to configure anything by hand.., if u want i can show my settings file like how its configured right now in my sublime.., should i ?.

tayelno commented 3 years ago

Yeah, allow this to be opt-in and it's reasonable to me 👍🏻

sorry now that I remember what was being achieved here, this doesn't need an opt-in configuration, as the current configuration only allow to pass an absolute path to typescript, so I think since people that are already using the plugin are split into two groups:

  1. using the plugin typescript (not specifying tsdk_path) -> will be using local tsdk if it exists and if not they will use the plugin tsdk
  2. using a specified tsdk_path -> not affected at all by the PR and it falls back to the one fetched inside the plugin itself, and I think most people would think that it goes.

I think your concern is about group number 1, but I was one of those people and thought that this work like other editors it uses:

  1. project(local) node_modules
  2. if not found -> global node_modules
  3. if not found -> plugin typescript

I am not saying your point is not valid but just that with how the users are currently using the plugin, and maybe there is something else I am missing but I think this is an improvement, as users would expect their project/local typescript to be used by the plugin, as they either specifically installed it for tooling or building the code, and I think in both cases the plugin should use that typescript version, and again maybe I am missing something or the whole point actually

orta commented 3 years ago

I don't disagree with the idea that someone could expect that, but I'm more worried from a security perspective. You could clone a repo with a poisoned version of the TypeScript dependency and simply opening that repo in Sublime would trigger the eval of whatever code they want.

If a user intentionally decides they think this trade-off is worth it, that is OK. It's not really OK by default.

I believe how this works in vscode is that if a downloaded project declares it wants to to use a local version of TypeScript in its settings, then you get a popup asking if you want to switch from the bundled version to the local copy.

sahilrajput03 commented 3 years ago

Amazing..!.

On Thu 27 May, 2021, 3:51 PM lastthyme, @.***> wrote:

@.**** approved this pull request.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/microsoft/TypeScript-Sublime-Plugin/pull/720#pullrequestreview-670006465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHQAJY22LB2P5YAJLRXBE4DTPYMLJANCNFSM4HOEO7LQ .