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

Support for bulk adding models/extraLibs #2030

Closed
StefanRetief opened this issue Jul 7, 2020 · 6 comments
Closed

Support for bulk adding models/extraLibs #2030

StefanRetief opened this issue Jul 7, 2020 · 6 comments

Comments

@StefanRetief
Copy link

StefanRetief commented Jul 7, 2020

monaco-editor version: 0.20.0
Browser: Chrome
OS: Windows
Playground code that reproduces the issue:

monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
    target: monaco.languages.typescript.ScriptTarget.ES2016,
    allowNonTsExtensions: true,
    moduleResolution: monaco.languages.typescript.ModuleResolutionKind.NodeJs,
    module: monaco.languages.typescript.ModuleKind.CommonJS,
    noEmit: true,
    typeRoots: ["node_modules/@types"]
});

for (let i = 0; i < 500; i++) {
    monaco.languages.typescript.typescriptDefaults.addExtraLib(
        `export declare function next${i}() : string`,
        `file:///node_modules/@types/external${i}/index.d.ts`
    )
    monaco.editor.createModel(`export declare function next${i}() : string`, "typescript", monaco.Uri.file(`/node_modules/@types/external${i}/index.d.ts`));
    
}

monaco.languages.typescript.typescriptDefaults.setDiagnosticsOptions({
    noSemanticValidation: false,
    noSyntaxValidation: false
})

var jsCode = `import * as x from "external1"
    const tt : string = x.next1();`;

monaco.editor.create(document.getElementById("container"), {
    model: monaco.editor.createModel(jsCode, "typescript", monaco.Uri.file("main.tsx")),
});

In the above sample, I can try to add 500 models to the editor. Normally, in small numbers, adding models isn't in issue, however, when adding more than about 100, I end up with "Loading..." when trying to hover over a function or import for about a minute or two.

@StefanRetief StefanRetief changed the title Proper method to load extraLibs for "Peek at definition" Support for bulk adding models/extraLibs Jul 8, 2020
@spahnke
Copy link
Contributor

spahnke commented Jul 8, 2020

I don't know what's the bottleneck here but for bulk adding extra libs there is monaco.languages.typescript.typescriptDefaults.setExtraLibs. So you could try to use that method to make the language server aware of all the files and then create the actuals models in a loop like you already do. Maybe that already makes a difference.

The relevant PR was microsoft/monaco-typescript#24

@StefanRetief
Copy link
Author

StefanRetief commented Jul 8, 2020

Hey @spahnke , thanks for the response. I've tried that and it seems to have the same effect. I believe the bottleneck is the createModel method. here's the code I tried it with:

monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
    target: monaco.languages.typescript.ScriptTarget.ES2016,
    allowNonTsExtensions: true,
    moduleResolution: monaco.languages.typescript.ModuleResolutionKind.NodeJs,
    module: monaco.languages.typescript.ModuleKind.CommonJS,
    noEmit: true,
    typeRoots: ["node_modules/@types"]
});
let extraLibs = []
for (let i = 0; i < 500; i++) {
    extraLibs.push(
        {
        content: `export declare function next${i}() : string`,
        filePath:  monaco.Uri.file(`/node_modules/@types/external${i}/index.d.ts`).toString(true)
        }
    );
}

monaco.languages.typescript.typescriptDefaults.setExtraLibs(extraLibs);

extraLibs.forEach((lib) => monaco.editor.createModel(lib.content, "typescript", monaco.Uri.parse(lib.filePath)))


monaco.languages.typescript.typescriptDefaults.setDiagnosticsOptions({
    noSemanticValidation: false,
    noSyntaxValidation: false
})

var jsCode = `import * as x from "external1"
    const tt : string = x.next1();`;

monaco.editor.create(document.getElementById("container"), {
    model: monaco.editor.createModel(jsCode, "typescript", monaco.Uri.file("main.tsx")),
});

I've found the implementation for the createModel method here: https://github.com/microsoft/vscode/blob/aef90f8fda20e6b7447710e311f95e7b4c6839c5/src/vs/editor/common/services/modelServiceImpl.ts#L486

and it appears to fire the onModelAdded event (https://github.com/microsoft/vscode/blob/aef90f8fda20e6b7447710e311f95e7b4c6839c5/src/vs/editor/common/services/modelServiceImpl.ts#L496) after each model. Maybe that has something to do with it? I wasn't able to track down the listeners for it just yet.

@spahnke
Copy link
Contributor

spahnke commented Jul 8, 2020

Running your code with

monaco.languages.typescript.typescriptDefaults.setDiagnosticsOptions({
    noSemanticValidation: true,
    noSyntaxValidation: true,
    noSuggestionDiagnostics: true
})

the language service reacts pretty fast so I don't think it's the model creation. It's more likely the 500 simultaneous requests to the language service to validate all those models.

The validation is done in this class
https://github.com/microsoft/monaco-typescript/blob/778ace10c0b702197eb06a3a1dc3b62b46eb5ceb/src/languageFeatures.ts#L90

more specifically here
https://github.com/microsoft/monaco-typescript/blob/778ace10c0b702197eb06a3a1dc3b62b46eb5ceb/src/languageFeatures.ts#L173-L181

if you want to play around with that. Unfortunately, I don't know another way to bulk add models and circumvent those language service requests. Good luck!

@StefanRetief
Copy link
Author

StefanRetief commented Jul 8, 2020

Ah that's probably it. That's interesting. Maybe I'll try looking into a work around to exclude validation of the models that share a url with an extralib. That definitely helps, thanks!

Edit: For anyone having the same issue, I've found that the best solution was to clone the monaco-typescript repo and customize how the validation works. I've adjusted it for myself so that any models that share the same url as an extralib will not be validated. Loading time has decreased significantly for situations where I have loaded a bunch of declaration files with that I'd also like to easily peek/go to definition with.

@orta
Copy link
Contributor

orta commented Jul 16, 2020

Cool, I was having this perf issue myself yesterday in microsoft/monaco-typescript#64 while looking to see if I can make Monaco models for all of the lib.d.ts files shipped in monaco-ts. Thanks for the explanation, makes sense

@StefanRetief
Copy link
Author

No worries. I should just mention, the method I used did have a side effect. eagerModelSync had to be set to false, otherwise I ran into performance issues with validating the model as the user types (i.e. marker data would lag behind whatever the user typed by about 1-2 seconds). Setting it to false corrected the issue and didn't seem to cause any other issues that I have found.

@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants