app playground: typescript multifile imports and node_modules support

[rubenfiszel]

Hi,

When using monaco-editor typescript worker, if you have packages put in the node_modules folder, the typescript worker is capable of resolving imports and showing related completion (https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAKjgQwM5wO4jgMyhbAIg2ADsATEYAG2oFoBja4AU1JkIChOsA6IA, this is how it works underneath, it automatically pull the packages in node_modules and then monaco worker do the rest). I cannot seem to reproduce the same behavior when using the app playground worker: https://typefox.github.io/monaco-languageclient/ghp_appPlayground.html (you have to copy paste folders in node_modules).

Other thing, not sure if it's a regression or not but I seem to remember that if you had two files:
a.ts:

export function foo() { return 42 }

If in b.ts you wrote

foo()

you could hover and it would suggest to create the import to a.ts automatically but I can't reproduce anymore in: https://typefox.github.io/monaco-languageclient/ghp_appPlayground.html

[rubenfiszel]

actually the issue is only if the files have never been opened. Once they are opened at least once then the relative import is possible.

[CGNonofr]

the monaco-editor typescript worker is only able to operate on open models, indeed

[rubenfiszel]

@CGNonofr that's not the case on normal vscode, is there a way to make it able to operate on all of node_modules and all files or at least a selection of file without having to open them in a tab?

[CGNonofr]

You don't need to have the file open in a tab for the model to be open, you just need a model reference to exist

VSCode doesn't use the monaco worker but the typescript language features default extension, which you can also use, but be aware of https://github.com/CodinGame/monaco-vscode-api/wiki/Troubleshooting#the-typescript-language-features-extension-is-not-providing-project-wide-intellisense

[rubenfiszel]

@CGNonofr How to create a model reference ? Is that different from having the file existing in a registered file system provider?

Also node_modules do not seem to work properly:

The folder of the package is well detected as noted by the auto-completion of the import name "windmill-client". However, the index.d.ts is not detected, and that's because I believe the path resolution is bugged. For some reasons, it tries to get it from "/file/ts-nul-authority/workspace/node_modules/windmill-client/dist/index.d.ts", when it should try to do it from "/workspace/node_modules/windmill-client/dist/index.d.ts"
I do not know where: /file/ts-nul-authority/ come from:

To reproduce, it should be sufficient to add:

    fileSystemProvider.registerFile(
        new RegisteredMemoryFile(
            vscode.Uri.file(
                "/workspace/node_modules/windmill-client/package.json"
            ),
            `{
    "name": "windmill-client",
    "version": "1.444.0",
    "main": "dist/index.js",
    "types": "dist/index.d.ts",
    "files": [
        "dist",
        "package.json"
    ]
}`
        )
    );
    fileSystemProvider.registerFile(
        new RegisteredMemoryFile(
            vscode.Uri.file(
                "/workspace/node_modules/windmill-client/dist/index.d.ts"
            ),
            `
            export declare function sayFoo(): string;`
        )
    );

to the official playground example.

I made a small video to illustrate

mov-2025-01-07--18-50-48.mp4

[CGNonofr]

How to create a model reference ?

monaco.editor.createModelReference or via the vscode api: vscode.workspace.openTextDocument() (which calls createModelReference itself)

Is that different from having the file existing in a registered file system provider?

Yes it is! the monaco worker has no idea about the virtual file system

if you want your filesystem to be well handled, you need the VSCode extension really

[rubenfiszel]

👍 Makes sense, thanks

you need the VSCode extension really

What do you mean by that exactly?

And any ideas about the node_modules not resolving properly ? It works on the monaco editor using file:// based models so my guess is that path resolution is not done either when being sent to tsserver or being received from tsserver. I found a reference to "ts-nul-authority" in that thread: microsoft/TypeScript#54926

[rubenfiszel]

@CGNonofr I've spawned up the debugger and the issue is not with ts-nul-authority.

I've compared the behavior of vcode.dev versus the app playground. It's enough to have in node_modules a folder X and an index.d.ts in it. On vscode.dev, if you have an import to X in another ts file, it will resolve but not on the app playground.

The difference mostly lies here: https://github.com/microsoft/vscode/blob/b0d6d34fbbdd73e273715a6f68e5dfbc7bf6147b/extensions/typescript-language-features/src/tsServer/bufferSyncSupport.ts#L551
At the point the tsserver URI resolves to a vscode resource, the syncedBuffers.allBuffers will contain an entry for index.d.ts in vscode.dev that match the tsserver fully qualified URI whereas monaco-vscode-api/app-playground doesn't. So the issues lies somewhere in the synchronization of this allBuffers. Still digging.

[kaisalmen]

@rubenfiszel and @CGNonofr mini-coi is used in the clangd example to solve the cross-origin problems with GitHub Pages. I did some local test with the dev server and the production preview build. Adding mini-coi to the app playground had not effect regarding the "ts-nul-authority" issue.

[CGNonofr]

👍 Makes sense, thanks

you need the VSCode extension really

What do you mean by that exactly?

I mean you stop using monaco-vscode-standalone-typescript-language-features and use monaco-vscode-typescript-language-features-default-extension instead

The first one is designed for monaco-editor, is very simple, has no idea about your virtual filesystem. The other one is the one used by vscode.dev

edit: but maybe it's already the case?

[kaisalmen]

edit: but maybe it's already the case?

@CGNonofr yes, the app_playground uses the monaco-vscode-typescript-language-features-default-extension and enables the ext host worker

[CGNonofr]

Ok so forgot about everything I've said on this whole issue 😅

[rubenfiszel]

@CGNonofr @kaisalmen I found it!
We need this commit on the source that build tsserver: microsoft/vscode@db1db15
Any chance you would be able to take the fix upstream?

[CGNonofr]

The fix was included in the v1.96, we need to update monaco-vscode-api to it, it'll come eventually