Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using default TypeScript tooling for specific dirs/files #314

Closed
lgarron opened this issue Feb 4, 2021 · 28 comments
Closed

Allow using default TypeScript tooling for specific dirs/files #314

lgarron opened this issue Feb 4, 2021 · 28 comments
Labels
enhancement New feature or request

Comments

@lgarron
Copy link

lgarron commented Feb 4, 2021

To Reproduce

  1. Write some TypeScript code for the client, e.g.:
document.body.appendChild(document.createElement("div"))

Expected behavior

I can write client-side code inside a project where I use Deno, using TypeScript defaults (e.g. using DOM APIs).

One of the great selling points of JS on the server is that it's possible to share client and server code. It makes a lot of sense to develop those in the same repo, but I can't figure out how to get back the normal TypeScript dev experience for my client code folders.

Observed behaviour

VSCode gets cranky if I try to use any DOM APIs.

  • All uses of the DOM API are marked as syntax errors.
  • If I try to write document.body by hand, the editor automatically replaces document with decodeURIComponent after I type the .. 😫

In combination with #313, this means I'm constantly cleaning up after the Deno extension. I also lose the regular benefits of TypeScript.

Versions

macOS 10.15.7
VSCode Version: 1.52.1
Deno extension version 2.3.3

deno --version
deno 1.7.1 (release, x86_64-apple-darwin)
v8 8.9.255.3
typescript 4.1.3

@roman-kirsanov
Copy link

roman-kirsanov commented Feb 11, 2021

I totally agree!

We need some kind of include/exclude options for this plugin.

In huge repos, when you wanna add some deno, but you still need to maintain the other typescript code that depends on node_modules, you cannot just enable this plugin, it will turn your project all red.

It's not always convenient to split up the code to different repos with different stacks, plugins etc

@kitsonk
Copy link
Contributor

kitsonk commented Feb 11, 2021

We will solve this in the 3.X version of the extension by supporting workspace folders, which allows per-folder vscode settings.

Ref: denoland/deno#8643

@kitsonk kitsonk added the enhancement New feature or request label Feb 11, 2021
@lgarron
Copy link
Author

lgarron commented Feb 13, 2021

We will solve this in the 3.X version of the extension by supporting workspace folders, which allows per-folder vscode settings.

Ref: denoland/deno#8643

😍

Is this close to being usable, or should I expect to work around it for a while?

@kitsonk
Copy link
Contributor

kitsonk commented Feb 13, 2021

We are aiming for a week or so, but workspace folders aren't part of yet. I will take a look and see how hard they are to implement.

@nettybun
Copy link

Relates to #297 as well

@lgarron
Copy link
Author

lgarron commented Mar 10, 2021

Relates to #297 as well

Indeed, so it seems!

We are aiming for a week or so, but workspace folders aren't part of yet. I will take a look and see how hard they are to implement.

Any luck on that? :-D

@kitsonk
Copy link
Contributor

kitsonk commented Mar 10, 2021

I will take a look and see how hard they are to implement.

Any luck on that? :-D

I looked, they are hard. They are likely to be after import completions.

@lgarron
Copy link
Author

lgarron commented Mar 10, 2021

I will take a look and see how hard they are to implement.

Any luck on that? :-D

I looked, they are hard. They are likely to be after import completions.

😢

Thanks for looking into it, though!
I'll keep an eye on this issue, since it's likely to make a big difference in when I can start adopting deno in some of my projects. ☺️

@what1s1ove
Copy link

will be looking forward it ❤️

@kitsonk
Copy link
Contributor

kitsonk commented May 2, 2021

It is currently what I am working on. Should be in Deno 1.10 at the latest 🤞

@borekb
Copy link

borekb commented May 12, 2021

Just want to note here that it doesn't quite work for us (I've tested with Deno 1.10.1 and v3.4.0 of the extension) and that we're not that keen on workspaces either 😄. More in #264 (comment) but generally, we'd prefer pattern-based enabling like this:

{
  "deno.enableForPaths": ["deno-scripts"]
}

More details in that linked comment.

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2021

Yeah, because of a bug it got pulled from the release. Will hopefully reland it in a couple days in a patch release.

@kitsonk kitsonk reopened this May 12, 2021
@AZMCode
Copy link

AZMCode commented May 27, 2021

Have you considered perhaps activating the extension per-file with a comment at the top of the file? Could be useful and non-intrusive.

@kitsonk
Copy link
Contributor

kitsonk commented May 27, 2021

@AZMCode that is not what vscode supports and it would be special to only Deno. The feature has been fixed in Deno main and will be available for the next release of Deno, at which point I will re-close this issue.

@ngryman
Copy link

ngryman commented May 28, 2021

@kitsonk Thanks for the update.

Pattern matching seems to be a quite popular request as well (1, 2, 3, 4).

Workspace folders are great to define settings at the project level. If a project is 100% in Deno this is fine. However, I don't think it's well suited for projects that are a mix of Deno and Node.js. Having multiple settings folders in the same project, only to enable/disable Deno, is a bit cumbersome and can quickly become a pain for big monorepos.

Would you consider adding support for pattern-matching at some point? Is there a way we could help?

@kitsonk
Copy link
Contributor

kitsonk commented May 28, 2021

Would you consider adding support for pattern-matching at some point? Is there a way we could help?

I don't think we should support something that vscode doesn't support natively. The language service protocol can accomodate this, but it is up to the IDE/editor to support it. vscode has a feature request open: microsoft/vscode#32693. So the best way to help is continue to voice support for the feature in vscode and the Deno language server should automatically work with it.

@borekb
Copy link

borekb commented May 28, 2021

@kitsonk I think that microsoft/vscode#32693 isn't the same request – I don't want to be writing multiple settings.json files, I want this scenario to be supported:

.
└── my-repo/
    ├── nodejs-server.ts
    └── deno-script.ts

@kitsonk
Copy link
Contributor

kitsonk commented May 28, 2021

I understand what you are requesting, but again, the language server protocol supports per file settings, it is up to the IDE to determine how those are set and resolved. If you want per file settings in vscode, and some sort of syntax to express it, I still feel it is a vscode request.

@borekb
Copy link

borekb commented May 28, 2021

I'm still trying to understand it. In #264 (comment), I asked this:

Is this something this extension can do on its own or does it also need support from the (built-in) TS/JS language server of VSCode?

So is the answer that it has to be supported by VSCode, or is there any way that the Deno extension could implement something like "deno.enableForPaths"? Maybe the actual project that needs to support this is TypeScript itself, which ships tsserver?

A bit unfortunate is that .ts file extension isn't really enough to tell the IDE which language server to run for it; does this problem also exist for other technologies, I don't know, like .py being interpreted as Python 2 in one folder and as Python 3 in another folder, and that it would be two different language servers? I don't know, just trying to find an analogy...

@kitsonk
Copy link
Contributor

kitsonk commented May 28, 2021

There are two parts to it, what the language server protocol supports per file configuration requests so the Deno lsp (in main) queries each file for its configuration settings and then manages that on a per request basis.

Because vscode has a built in TypeScript language service as well, we have to "mute" that when Deno is enabled. We "hack" the built in language service via a TypeScript language server plugin and send information about each file see is enabled for Deno.

It is not a technical impossibility to create a feature like "deno.enableForPaths" it is that it adds a lot of complexity in addition to supporting what is built into the language server protocol, and ultimately is the domain of the IDE to provide the settings of the file. Multi-Root Workspaces is how vscode currently varies configuration within a workspace. If you want more granular IDE configuration, it is best to focus on requesting that from vscode, as the language server protocol already supports it, and instead of a specific "hack" for Deno, it would allow all extensions to benefit.

Also, the closer the Deno language server sticks to the protocol, the more it will be able to benefit other IDEs without having to "hack" around it. Again, the protocol supports an IDE providing per resource configuration, so why provide a "hack" to subvert that with its own semantics.

@borekb
Copy link

borekb commented May 28, 2021

Multi-root workspaces aren't the only way that VSCode does per-resource customization though, for example, I have this in my settings.json:

{
  "editor.wordBasedSuggestions": true,
  "[typescript]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode",
    "editor.wordBasedSuggestions": false
  },
  "[typescriptreact]": {
    "editor.wordBasedSuggestions": false
  },
}

Another example, this time by an extension:

{
  "spellright.spellContext": "body comments strings",
  "spellright.spellContextByClass": {
    "markdown": "body",
    "typescript": "comments",
    "jsonc": "comments",
    "html": "body",
    "typescriptreact": "body comments",
    "php": "comments"
  }
}

It's not a perfect analogy but I'm trying to advocate for a solution that could work soon-ish and would not depend on VSCode / tsserver (I watch issues such as the one you linked above and they move very slowly, if at all).

BTW, looking at the examples above, maybe there could be typescriptdeno "language" similar to typescriptreact? Not sure if it would help but from a user perspective, "Deno TypeScript" is a bit different from "regular TypeScript".

@kitsonk
Copy link
Contributor

kitsonk commented May 28, 2021

The first example is provided by vscode and the Deno language server in main supports this.

The second is exactly what I am trying to avoid, extension "hacks". They could have simply leveraged the feature above, but they chose to create their own syntax, and now they have to support it.

BTW, looking at the examples above, maybe there could be typescriptdeno "language" similar to typescriptreact?

No, those file types are determined by the editor and are not "customisable". There are a few issues in vscode where people want to vary them based on specific extensions (like .js versus .mjs) and the vscode team has refused.

@MKRhere
Copy link

MKRhere commented Jun 3, 2021

So as I understand, having a mix of node and deno scripts in a repository is not supported until VSCode supports it? For example, there's no way to enable Deno lsp for scripts/ while using Typescript for src/?

@kitsonk
Copy link
Contributor

kitsonk commented Jun 3, 2021

@MKRhere that would be a multi-root workspace: https://code.visualstudio.com/docs/editor/multi-root-workspaces where you would add a scripts and tsc folder to your workspace.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 3, 2021

Deno 1.10.3 is available which supports the workspace folders/per resource configuration, so closing this issue again.

@kitsonk kitsonk closed this as completed Jun 3, 2021
@MKRhere
Copy link

MKRhere commented Jun 3, 2021

That's great, thank you!

LumaKernel added a commit to LumaKernel/coc-denoland that referenced this issue Aug 7, 2021
* feat: code lens for references (denoland#308)

* feat: disable most of builtin language service when deno enabled (denoland#307)

* 0.0.8

* feat: add applyCodeActionCommand command (denoland#312)

* fix: remove deno/applyCodeActionCommand (denoland#315)

* New high-res logo (denoland#274)

* feat: add implementations code lens configuration option (denoland#319)

* feat: add initialize workspace command (denoland#316)

* feat: support deno cache quick fix (denoland#322)

* chore: add screenshot to README (denoland#323)

* 0.0.9

* 0.0.10

* fix: typo in init command (denoland#327)

* feat: add a welcome screen for extension (denoland#329)

* feat: use preview instead of display for status (denoland#330)

* chore: README improvements (denoland#331)

* Release 3.0.0, canary is now main (denoland#332)

* 3.0.1

* Fix typo (denoland#337)

Grammar + context

* chore: activate extension on command (denoland#336)

* docs: recommend import_map.json instead of import-map.json (denoland#340)

Resolves denoland#338

* chore: move Releases.md to CHANGELOG.md for better marketplace integration (denoland#344)

Closes denoland#342

* feat: add deno.path setting (denoland#350)

* 3.1.0

* feat: read-add debug support (denoland#351)

Co-authored-by: CGQAQ <[email protected]>

* feat: add settings to affect completions (denoland#368)

* fix: manual `deno` command resolution on windows. (denoland#367)

Fixes denoland#361

* 3.2.0

* feat: support for relative path resolution (using workspaces) in deno.path (denoland#381)

Co-authored-by: Kitson Kelly <[email protected]>

* feat: add version notification message (denoland#383)

* feat: add restart language server command (denoland#385)

Resolves denoland#372

* feat: add support for import registry completions (denoland#380)

* 3.3.0

* typo in ImportCompletions.md (denoland#390)

* fix: activate on reloadImportRegistries command (denoland#407)

Fixes: denoland#394

* feat: handle per resource configuration (denoland#411)

Requires Deno with denoland/deno#10488.

Ref: denoland#348 
Resolves: denoland#314
Resolves: denoland#297

* feat: add internalDebug config flag (denoland#406)

Also integrate upstream formatting changes in deno fmt.

Ref: denoland/deno#10368

* 3.4.0

* feat: recognise json(c) & markdown files (denoland#404)

* 3.5.0

* docs: fix broken link in README (denoland#426)

* feat: support registry auto discovery (denoland#427)

* fix: bump semver of extension (denoland#429)

* 3.5.1

* feat: add support for tasks and test code lens (denoland#436)

* 3.6.0

* fix: update packaging and pin vsce version (denoland#440)

Fixes denoland#439

* 3.6.1

* feat: add support for import map in test code lens (denoland#446)

* fix: activate extension on markdown / json / jsonc (denoland#447)

* fix: setting then clearing "deno.path" config should not use empty string for path (denoland#452)

* fix: better handling when language server fails to start (denoland#454)

* 3.7.0

* fix: remove trailing slash in example (denoland#471)

* chore: remove test header from bug report issue template (denoland#479)

* feat: add ability to set cache directory in settings (denoland#477)

Closes denoland#287

* fix: properly handle plugin configuration at startup (denoland#474)

Fixes denoland#473

Co-authored-by: Kitson Kelly <[email protected]>
Co-authored-by: Luca Casonato <[email protected]>
Co-authored-by: Kirill Reunov <[email protected]>
Co-authored-by: Liam Murphy <[email protected]>
Co-authored-by: Ryan Dahl <[email protected]>
Co-authored-by: Jesse Jackson <[email protected]>
Co-authored-by: CGQAQ <[email protected]>
Co-authored-by: David Sherret <[email protected]>
Co-authored-by: Hector Menendez <[email protected]>
Co-authored-by: Heyward Fann <[email protected]>
Co-authored-by: Satya Rohith <[email protected]>
Co-authored-by: yaegassy <[email protected]>
Co-authored-by: Cedric Vangout <[email protected]>
@johnspurlock
Copy link

Deno 1.10.3 is available which supports the workspace folders/per resource configuration

Can you please reopen this issue? It doesn't support deno.config in the workspace folder settings overrides yet, which would be super useful for developing a deno client/server app in the same multi-root workspace!

@bombillazo
Copy link

Hello, how can one the deno extension by default? I constantly get deno errors and continue having to add "deno.enable": false to my vscode projects. this is sometimes not permitted when collaborating on forked repos...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests