[HTML] Support the tsserver resolving files relative to the html file #121517

orta · 2021-04-16T21:55:03Z

I'm not sure it completes every feature requested in #26338 so I'm not marking this PR as fixing it, it might, but I don't have enough tests to verify it - but it does solve the one I really wanted: resolving JS Doc import types inside the script tags.

This PR switches just the TSServer to support reaching into the file-system in HTML-language-features giving TypeScript the chance to handle types across file boundaries.

Auto-complete for imports

Grabbing globals from `@types`

orta

The one thing which I'd love some advice on is how I should invalidate the files which TypeScript sees. E.g. right now if I edit the exports for a module referenced in the script, the jsLanguageService doesn't know the file content needs updating.

Is there a general file watcher I can hook into for the workspace - where I can then tell the language service to clear the cache.

orta · 2021-04-16T21:55:57Z

extensions/html-language-features/server/src/modes/javascriptMode.ts

+	// Both \ and / must be escaped in regular expressions
+	return newPath.replace(new RegExp('\\' + sep, 'g'), '/');
+}
+


Key part 1: file://a/b/c.html -> /a/b/c.html so that TypeScript is always working with pure file paths

orta · 2021-04-16T21:57:36Z

extensions/html-language-features/server/src/modes/javascriptMode.ts

 function getLanguageServiceHost(scriptKind: ts.ScriptKind) {
 	const compilerOptions: ts.CompilerOptions = { allowNonTsExtensions: true, allowJs: true, lib: ['lib.es6.d.ts'], target: ts.ScriptTarget.Latest, moduleResolution: ts.ModuleResolutionKind.Classic, experimentalDecorators: false };

 	let currentTextDocument = TextDocument.create('init', 'javascript', 1, '');
+	let currentWorkspace: Workspace = undefined!;


Key part 2: Using the local workspace to act as the TS project root

This could maybe instead be based solely on the input .html file instead? It felt preferable to use the workspace, but I could understand opting for the folder the html lives in instead.

orta · 2021-04-16T21:58:36Z

extensions/html-language-features/server/src/modes/javascriptMode.ts

+			readFile: ts.sys.readFile,
+			readDirectory:  ts.sys.readDirectory,
+			directoryExists: ts.sys.directoryExists,
+			getDirectories: ts.sys.getDirectories,


Key part 3: Reuse the TypeScript fs functions to let this TSServer read from the fs

orta · 2021-04-16T22:00:23Z

extensions/html-language-features/server/src/modes/javascriptMode.ts

+			const filePath = deschemeURI(jsDocument.uri);
+
+			const syntaxDiagnostics: ts.Diagnostic[] = languageService.getSyntacticDiagnostics(filePath);
+			const semanticDiagnostics = languageService.getSemanticDiagnostics(filePath);
 			return syntaxDiagnostics.concat(semanticDiagnostics).map((diag: ts.Diagnostic): Diagnostic => {


The changes here are adding , workspace) and then using filePath whenever the LSP talks to the language service explicitly.

This is the same pattern for the rest of the file

ObscurusGrassator · 2021-04-17T10:32:58Z

Hi. Unfortunately, the Microsoft team does not want to address this issue intentionally, so merging is unlikely. They ignore it for 4 years. Please make an extension to vscode. And thank you for working to solve this problem. :-)

orta · 2021-04-17T10:39:17Z

That's not a very helpful comment, the vscode team's project has a massive scope and the issues for this are both backlog'd and still open. This is generally a good indicator that they would accept a small and tested 3rd party PR for those items. Though, FWIW, I am also a Microsoft employee and I've definitely got backlog issues far far longer.

aeschli · 2021-04-20T06:52:01Z

Thanks for helping out, @orta ! Yes file watching is needed and access to the unsaved text documents. -> the html server needs to subscribe for open JS documents as well.

glaukiol1

lol ive always wanted this, tired of my inline js being plain text. Thanks for making this fix

orta · 2021-05-02T07:49:06Z

OK, I've got this green and seems to work exactly how I expect overall.

Right now, it uses mtime on app js/ts files to determine if it needs to be re-grabbed, which is potentially little resource wasteful but also can handle all of the different possible files which people use in JS (think packagers like webpack which allow for importing a .css files into a JS file)

That said, I'd like to give it one more look to see what expanding using the watcher for html files looks like. So this is good enough if you want to try locally, but I'd like to at least give that one more run before being considered fully ready for review.

aeschli · 2021-05-10T11:23:29Z

Thanks @orta!

We'd still need to look into unsaved files.
The HTML server can also run in a web worker (for VSCode running fully in the browser)' and on virtual resources.
There we can't use fs. The implementation uses a file system abstraction named RequestService (not the best name, I'll will improve it at some point). The request service either uses node-fs or forwards the requests to the file system provider in the VS Code API.
Now, this is problematic for to use with TypeScript as TypeScript depends on synchronous file accesses. The RequestService is async.

orta · 2021-05-11T18:22:12Z

I think it's safe to say that it's unlikely the tsserver api will be async in a reasonable timeframe (if ever) microsoft/TypeScript#29361 tracks it. I am kinda interested in how that works out for the official TS extension in Code, though the implementation differs because that is process separated and can probably have different constraints.

So far, I think this leaves us with the following options:

Use the RequestService API and try to pre-cache all possible file contents ahead of time (which is what deno does) - which means recursively requesting every possible file (which would include all the .d.ts files from the node modules etc) so that we can present it sync... Which is kind of wild in the npm ecosystem.
The "give-up and revert" to the prior behavior of not looking outside the current file on a FS without synchronous access

orta · 2021-06-04T07:35:46Z

I think making this turn itself off elegantly (and basically providing the same experience there is today) in the cases where we don't have sync access to a filesystem probably makes the most sense

orta · 2021-09-06T15:24:50Z

Is there a recommended system-level way to detect we're in a browser and then I can disable the TSServer?

aeschli · 2021-09-07T07:54:07Z

You could look at the schema of the workspace folder(s) and if it is not file, don't use fs. That solution also helps VS Code opened on virtual folders

kyr0 · 2021-11-22T10:31:57Z

@orta Great PR! I really hope this one is being merged sooner or later. Until then: Have you released any extension based on this? I'd really like to have this instead of the internal one.

orta · 2021-11-22T19:24:30Z

This is pretty close now, it also checks for a local folder in the workspace and only allows runs in that case. It can now be bundled for web and doesn't have runtime issues:

Just gotta get it green and make sure I've not broken the local version in the process

kyr0 · 2021-11-23T11:05:32Z

Wow, thank you so much already!

kyr0 · 2021-12-05T09:38:37Z

@orta Is there anything, one could help with?

orta · 2021-12-05T11:58:57Z

Nah, I've just not had a chance to get back to the PR, you're welcome to rebase and get the tests green - that's all there is to do atm. I just need to find the time and inclination to wrap it up

kyr0 · 2021-12-06T10:56:00Z

@orta Ah cool, thanks for the quick response. Let me see if I can manage it. This feature is very helpful for tooling that is focusing on "back to the roots" workflows/architectures (see Island Arch https://jasonformat.com/islands-architecture/) or web components and such; making the use of <script> more and more popular again; also given that we can now work with most of the modern APIs and language features at a 95% browser compatibility range and thus, folks are returning to Vanilla JS/TS more often. I'm working on an open source SSG solution which is also promoting <script> for simple use-cases. The developer experience in VS Code is def. dependent on this feature. I think some Snowpack / Astro.build folks would also be happy to have it (see https://docs.astro.build/core-concepts/component-hydration/ -- last section)

orta · 2021-12-07T11:45:37Z

Yeah, I originally started this PR because I wanted to use this in my snowpack/vite apps

orta · 2021-12-07T15:55:28Z

The CI fail is unrelated, this ins rebased and I added a test for the vfs - I'm a little surprised that I don't see the changes when I run the extension in dev mode, and I can't attach to the LSP. So I'm not sure if I just have a borked local vscode (which could be very possible) or it only works in tests and not reality :D

So I'll try look on another computer to validate

kyr0 · 2021-12-07T23:14:17Z

Hey @orta, very nice that you've spent some time on this again so quickly :) It would def. take me a while to get at speed with this. I've just cloned, compiled, ran the tests etc. :) It was a liiitle bit of a bumpy road for me as I'm dealing with the vscode codebase for the first time :D So, yeah, I have a bit of experience with the tooling etc. and just delved into the GitHub actions command to pave my way thru the jungle of what I need to get everything up and running locally in dev mode ;)

Finally, I've got it working after an hour or so :))

Here are my current test results:

Verified to actually work in reality, yay! 💯

Collides with this branch of reality :D

Auto-complete on (local) file paths

Auto-complete on import

In case you'd like me to test more you, I'm very open to do so if you'd like to give me some exact instructions.

Thanks alot!
Aron

orta · 2021-12-08T11:21:29Z

Yeah, cool, that is basically the same experience as without this PR, thanks.

So, I've definitely broke it in file:// projects when making sure vfs ones don't get the behavior 👍🏻

dpikt · 2022-07-20T16:15:22Z

@orta I think I've identified the issue - the fileExists: currentWorkspace && ts.sys.fileExists check is disabling the behavior in all cases, not just vfs. It could probably be fixed by doing the check within the function body rather than before the function is assigned. I'd be happy to make a change to get this PR over the finish line, if you'd like. Also, as you pointed out, the tests don't catch this and need to be updated. (They're also not testing that the functionality is disabled for vfs correctly).

Edit: To be clear I'm not sure if fileExists specifically is the problem, but those conditionally assigned methods in general aren't working.

dpikt · 2023-09-20T15:45:36Z

Heads up, I've opened a PR that should address this same issue: #193590

vivodi · 2024-12-29T16:33:12Z

Is this still relevant?

orta commented Apr 16, 2021

View reviewed changes

vscode-triage-bot assigned aeschli Apr 16, 2021

orta force-pushed the html-tsserver-fs branch from ac58193 to 511d817 Compare April 17, 2021 05:18

glaukiol1 previously approved these changes Apr 30, 2021

View reviewed changes

orta force-pushed the html-tsserver-fs branch from 2338d9e to b93b60b Compare May 2, 2021 07:16

aarsxx previously approved these changes May 5, 2021

View reviewed changes

orta mentioned this pull request May 11, 2021

TypeScript and <script type="module"></script> microsoft/TypeScript#13422

Closed

orta force-pushed the html-tsserver-fs branch from b93b60b to 10a4096 Compare November 22, 2021 18:26

orta added 6 commits December 7, 2021 13:34

Adds the ability to resolve @types and JSDoc imports in inline <scripts>

989d156

Improve tests

9b71947

Support windows filepaths

12389b3

Use mtime to figure out what the version of a TypeScript import file is

c2f8374

html-lang: Prefer full urls in JS auto-complete in script tags

a9b02de

Only use the sync TSServer when on a file:// workspace

a787a13

Get tests green

e4d3ca9

orta force-pushed the html-tsserver-fs branch from 10a4096 to e4d3ca9 Compare December 7, 2021 15:19

alexdima dismissed stale reviews from aarsxx and glaukiol1 via e4d3ca9 May 5, 2022 09:18

dpikt mentioned this pull request Sep 20, 2023

Resolve JS imports from html script tags #193590

Open

friuns2 approved these changes Mar 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HTML] Support the tsserver resolving files relative to the html file #121517

[HTML] Support the tsserver resolving files relative to the html file #121517

orta commented Apr 16, 2021 •

edited

Loading

orta left a comment

orta Apr 16, 2021

orta Apr 16, 2021

orta Apr 16, 2021

orta Apr 16, 2021

orta Apr 16, 2021

ObscurusGrassator commented Apr 17, 2021

orta commented Apr 17, 2021 •

edited

Loading

aeschli commented Apr 20, 2021

glaukiol1 left a comment

orta commented May 2, 2021

aeschli commented May 10, 2021

orta commented May 11, 2021

orta commented Jun 4, 2021

orta commented Sep 6, 2021

aeschli commented Sep 7, 2021

kyr0 commented Nov 22, 2021

orta commented Nov 22, 2021

kyr0 commented Nov 23, 2021

kyr0 commented Dec 5, 2021

orta commented Dec 5, 2021

kyr0 commented Dec 6, 2021

orta commented Dec 7, 2021

orta commented Dec 7, 2021

kyr0 commented Dec 7, 2021

orta commented Dec 8, 2021

dpikt commented Jul 20, 2022 •

edited

Loading

dpikt commented Sep 20, 2023 •

edited

Loading

vivodi commented Dec 29, 2024

[HTML] Support the tsserver resolving files relative to the html file #121517

Are you sure you want to change the base?

[HTML] Support the tsserver resolving files relative to the html file #121517

Conversation

orta commented Apr 16, 2021 • edited Loading

Auto-complete for imports

Grabbing globals from @types

orta left a comment

Choose a reason for hiding this comment

orta Apr 16, 2021

Choose a reason for hiding this comment

orta Apr 16, 2021

Choose a reason for hiding this comment

orta Apr 16, 2021

Choose a reason for hiding this comment

orta Apr 16, 2021

Choose a reason for hiding this comment

orta Apr 16, 2021

Choose a reason for hiding this comment

ObscurusGrassator commented Apr 17, 2021

orta commented Apr 17, 2021 • edited Loading

aeschli commented Apr 20, 2021

glaukiol1 left a comment

Choose a reason for hiding this comment

orta commented May 2, 2021

aeschli commented May 10, 2021

orta commented May 11, 2021

orta commented Jun 4, 2021

orta commented Sep 6, 2021

aeschli commented Sep 7, 2021

kyr0 commented Nov 22, 2021

orta commented Nov 22, 2021

kyr0 commented Nov 23, 2021

kyr0 commented Dec 5, 2021

orta commented Dec 5, 2021

kyr0 commented Dec 6, 2021

orta commented Dec 7, 2021

orta commented Dec 7, 2021

kyr0 commented Dec 7, 2021

orta commented Dec 8, 2021

dpikt commented Jul 20, 2022 • edited Loading

dpikt commented Sep 20, 2023 • edited Loading

vivodi commented Dec 29, 2024

orta commented Apr 16, 2021 •

edited

Loading

Grabbing globals from `@types`

orta commented Apr 17, 2021 •

edited

Loading

dpikt commented Jul 20, 2022 •

edited

Loading

dpikt commented Sep 20, 2023 •

edited

Loading