microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.05k stars 29.22k forks source link

JS/TS Inlay hints settings should be scoped to resource #140414

Closed bpasero closed 2 years ago

bpasero commented 2 years ago

I use a multi-root workspace:

{
    "folders": [
        {
            "path": "monaco"
        },
        {
            "path": "vscode-docs"
        }
    ],
    "settings": {
        "files.associations": {
            "cglicenses.json": "jsonc"
        },
        "typescript.tsdk": "monaco/node_modules/typescript/lib",
        "emmet.excludeLanguages": [],
        "git.ignoreLimitWarning": true,
        "remote.extensionKind": {
            "msjsdiag.debugger-for-chrome": "workspace"
        },
        "typescript.tsc.autoDetect": "off"
    }
}

I do not see inlay hints in vscode (monaco is the vscode repo) even though they are enabled in workspace settings for vscode.

jrieken commented 2 years ago

@bpasero I cannot reproduce this... I have a dummy workspace that contains vscode, monaco-editor, and another folder and all works as expected, e.g by default inlays show everywhere, when disabling inlay per user-settings the vscode-folder still shows them. Anything else would also be surprising as there is no special workspace logic for inlay hints.

Do you still see this? Iff so, can you set a breakpoints here: https://github.com/microsoft/vscode/blob/1498d0f34053f854e75e1364adaca6f99e43de08/src/vs/editor/contrib/inlayHints/browser/inlayHintsController.ts#L128

bpasero commented 2 years ago

Yeah, it goes past that breakpoint, it's something else. This is quite bizarre, I am able to reproduce with a fresh user data dir and fresh extensions dir on insiders even with just 1 folder (called monaco our repo) transitioning into multi root workspace:

Recording 2022-01-24 at 16 46 26

When writing the workspace file we do migrate all workspace settings over into the multi root file, not sure that is relevant?

The full file ends up being:

{
    "folders": [
        {
            "path": "../Development/Microsoft/monaco"
        }
    ],
    "settings": {
        "files.associations": {
            "cglicenses.json": "jsonc"
        },
        "typescript.tsdk": "node_modules/typescript/lib",
        "git.ignoreLimitWarning": true,
        "remote.extensionKind": {
            "msjsdiag.debugger-for-chrome": "workspace"
        },
        "typescript.tsc.autoDetect": "off",
        "testing.autoRun.mode": "rerun",
        "explorer.experimental.fileNesting.patterns": {
            "*.js": "$(capture).*.js",
            "bootstrap.js": "bootstrap-*.js"
        }
    }
}
jrieken commented 2 years ago

This is quite bizarre, I am able to reproduce with a fresh user data dir and fresh extensions dir on insiders even with just 1 folder (called monaco our repo) transitioning into multi root workspace:

And then still going pass that extension point? Maybe the extension host configuration object isn't updated properly so that TypeScript inlay hints aren't enabled...

bpasero commented 2 years ago

Btw this reproduces from then on also after a window reload, so it is not related to entering the workspace. Maybe we can debug on my machine.

bpasero commented 2 years ago

It seems to only hit this method once when I open an editor and early return:

Recording 2022-01-24 at 16 57 26 (1)

jrieken commented 2 years ago

hm, seems like no inlay hints provider registers for you... (or source maps are wrong...)

bpasero commented 2 years ago

@mjbvz it looks like typescript is not passing in the resource scope when resolving the configuration here:

https://github.com/microsoft/vscode/blob/8ef8933a1af8bc3613dcc375d7483c2fd6da6add/extensions/typescript-language-features/src/languageFeatures/inlayHints.ts#L100

Even though it is marked as resource setting:

https://github.com/microsoft/vscode/blob/e15397d2bb4625eb8ab18e69760b8b05e9c4abe7/extensions/typescript-language-features/package.json#L320

Not sure there are other settings that have this problem?

mjbvz commented 2 years ago

No this should be a unique case

Actually applies to a few other settings (not everyone using requireConfiguration is working with resource level configurations but I think at least a few are): https://github.com/microsoft/vscode/blob/48c560d788650f3d0ffaa4da38b18729277f77c7/extensions/typescript-language-features/src/utils/dependentRegistration.ts#L92