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

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

Open
1 of 2 tasks
kasper573 opened this issue Dec 16, 2022 · 3 comments
Open
1 of 2 tasks
Labels
bug Issue identified by VS Code Team member as probable bug typescript typescript-multifile

Comments

@kasper573
Copy link

kasper573 commented Dec 16, 2022

Reproducible in vscode.dev or in VS Code Desktop?

  • Not reproducible in vscode.dev or 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'.

@kasper573 kasper573 changed the title [Bug] Updating an existing file using setExtraLibs has no impact [Bug] Consecutive calls to setExtraLibs fails to update an inferface definition Dec 16, 2022
@kasper573 kasper573 changed the title [Bug] Consecutive calls to setExtraLibs fails to update an inferface definition [Bug] Consecutive calls to setExtraLibs fails to update an interface definition Dec 16, 2022
@pennywort
Copy link

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
Copy link
Author

kasper573 commented Dec 16, 2022

@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
Copy link

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.

@hediet hediet added info-needed Issue requires more information from poster and removed info-needed Issue requires more information from poster labels Jan 9, 2023
@hediet hediet added bug Issue identified by VS Code Team member as probable bug typescript typescript-multifile labels Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug typescript typescript-multifile
Projects
None yet
Development

No branches or pull requests

3 participants