#538 Automatically check if node_modules/typescript/lib exists

[tayelno]

Fixes #538

• Tested on macOS Mojave, can't test it on anything else
  Inputs are welcome 😄

[kylebebak]

This would be a great improvement.

[orta]

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]

Make this feasible please!

[sahilrajput03]

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!!

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

[tayelno]

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]

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

[sahilrajput03]

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]

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]

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]

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 <#720 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHQAJY22LB2P5YAJLRXBE4DTPYMLJANCNFSM4HOEO7LQ> .