microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.35k stars 3.59k forks source link

[Bug] Consecutive calls to `setExtraLibs` fails to update an interface definition #3465

Open kasper573 opened 1 year ago

kasper573 commented 1 year ago

Reproducible in vscode.dev or in VS Code Desktop?

Reproducible in the monaco editor playground?

Monaco Editor Playground Code

function updateModelDefinition(propertyName) {
    var filePath = 'ts:filename/model.d.ts';
    var content = `interface Model { ${propertyName}: string }`;
    monaco.languages.typescript.typescriptDefaults.setExtraLibs([{filePath, content}]);
}

monaco.editor.create(document.getElementById('container'), {
    value: `const test: Model = {}`,
    language: 'typescript'
});

// This call works as expected
updateModelDefinition("immediate")

// Bug: Any future calls to setExtraLibs seem to be ignored
setTimeout(() => updateModelDefinition("afterTwoSeconds"), 2000);

setTimeout(() => updateModelDefinition("afterFiveSeconds"), 5000);

Reproduction Steps

Click run in the playground and wait a few seconds to let the timeouts fire and observe that the consecutive calls to setExtraLibs does not have the expected effect.

Actual (Problematic) Behavior

The editor keeps using the code provided by the first call to setExtraLibs, even if its called multiple times with new input.

Observe that the error in the playground example is incorrect: Property 'immediate' is missing in type '{}' but required in type 'Model'.

Expected Behavior

The editor should respect the most recent call to setExtraLibs.

The playground example should after 2 seconds yield the error: Property 'afterTwoSeconds' is missing in type '{}' but required in type 'Model'.

The playground example should after 5 seconds yield the error: Property 'afterFiveSeconds' is missing in type '{}' but required in type 'Model'.

pennywort commented 1 year ago

setExtraLibs has this in description

Remove all existing extra libs and set the additional source files to the language service.

You do not change the filepath while updating libs, so the lib is added as new version of existing one. Check it by adding this code to your update method: console.log(monaco.languages.typescript.typescriptDefaults._extraLibs) and see version: 1, version: 2.

If you change var filePath = 'ts:filename/model.d.ts'; to var filePath = 'lib:' + propertyName + '/model.d.ts'; then your calls will affect.

I don't use extra libs, I just add my files as new models and everything works fine. Maybe you need to think about this way.

kasper573 commented 1 year ago

@pennywort I tried your suggestion and while it's true your approach makes setExtraLibs have an effect, it's still seems to not work properly. When I try your suggestion, monaco seems to combine some of the calls to setExtraLibs, which is even more confusing. The documentation says that setExtraLibs clears all previous libs and replaces with the new input, but that is clearly not happening in either my original post, or with your approach.

With your approach I get this error instead: image

Which is very strange. It remembers the first call, and the last, but not the intermediate. Monaco behaves as if there's two libs registered with the same interface, and merges them. But that doesn't make sense, since setExtraLibs is supposed to clear previous libs, we should only end up with a single interface definition.

Btw, is there anywhere I can read about how to work with models and extra libs? The monaco editor website only has examples and a pretty slim api doc. It's really hard to grasp how you're supposed to use certain APIs.

pennywort commented 1 year ago

Yes, this strange effects are fixed by adding model for each extra lib (libs are used for d.ts accordingly to docs). By the way, this is recommended in playground:

var libUri = 'ts:filename/facts.d.ts';
monaco.languages.typescript.javascriptDefaults.addExtraLib(libSource, libUri);
// When resolving definitions and references, the editor will try to use created models.
// Creating a model for the library allows "peek definition/references" commands to work with the library.

If you add the model for each lib, everything will work as you expect.

If you want dynamically manipulate your libs you may need this:

const extraLibs = [];
function updateModelDefinition(propertyName) {
    var filePath = `ts:${propertyName}/model.d.ts`;
    var content = `interface Model { ${propertyName}: string }`;
    const extraLib = monaco.languages.typescript.typescriptDefaults.addExtraLib(content, filePath);
    const model = monaco.editor.createModel(content, 'typescript', monaco.Uri.parse(filePath));
    model.removeMe = true;
    extraLib.removeMe = true;
    extraLibs.push(extraLib);
}

monaco.languages.typescript.typescriptDefaults.setEagerModelSync(true);

monaco.editor.create(document.getElementById('container'), {
    value: `const test: Model = {}`,
    language: 'typescript'
});

setTimeout(() => {
    console.log('now removing models and libs...');
    console.log('models count: ', monaco.editor.getModels().length);
    monaco.editor.getModels().forEach(model => model.removeMe && model.dispose());
    extraLibs.forEach(lib => lib.removeMe && lib.dispose());
    console.log('models count: ', monaco.editor.getModels().length);
}, 10000)

// This call works as expected
updateModelDefinition("immediate")

// Bug: Any future calls to setExtraLibs seem to be ignored
setTimeout(() => {
    console.log(monaco.languages.typescript.typescriptDefaults._extraLibs)
    updateModelDefinition("afterTwoSeconds")
}, 2000)

setTimeout(() => {
    console.log(monaco.languages.typescript.typescriptDefaults._extraLibs)
    updateModelDefinition("afterFiveSeconds")
}, 5000);

You may play with commenting line where models are added and see the result of this. So I can't tell you how to use and combine models, libs and whatever correctly. I'm flying blind when working with monaco. It would be great to have a video guide or an article with good practices and approaches.