microsoft / monaco-editor

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

[Bug] 0.32.0 Regression: setModelLanguage no longer works with mime types #3170

Closed Prinzhorn closed 1 year ago

Prinzhorn commented 2 years ago

Reproducible in vscode.dev or in VS Code Desktop?

Reproducible in the monaco editor playground?

Monaco Editor Playground Code

const editor = monaco.editor.create(document.getElementById('container'), {
    value: "function hello() {\n\talert('Hello world!');\n}",
    language: 'plaintext'
});

monaco.editor.setModelLanguage(editor.getModel(), 'text/javascript');

Note that this works and has always worked (using mime as language):

const editor = monaco.editor.create(document.getElementById('container'), {
    value: "function hello() {\n\talert('Hello world!');\n}",
    language: 'text/javascript'
});

Actual Behavior

Language does not change / syntax not highlighted

Expected Behavior

Language changes and syntax highlighting updates

Additional Context

I've wrapped Monaco in a component and use the language prop in the monaco.editor.create call. When the language prop changes I call monaco.editor.setModelLanguage(editor.getModel(), language);. This was working until 0.32.0.

Do I need to use updateOptions? I think I went with this approach because I wasn't trusting updateOptions to only do minimal work (e.g. not create a new model just because I update the language).

Prinzhorn commented 2 years ago

Do I need to use updateOptions? I think I went with this approach because I wasn't trusting updateOptions to only do minimal work (e.g. not create a new model just because I update the language).

NVM, doesn't work either. In fact it seems like updateOptions with a mime type as language didn't even work before 0.32.0. So there's definitely an inconsistency here and something is broken. Maybe that's why I didn't use updateOptions back when I wrote this code.

hediet commented 2 years ago

I debugged the code and verified that it does not use the language id to lookup in the mime type registry.

I cannot verify the regression though.

@alexdima did something change here?

Prinzhorn commented 2 years ago

I cannot verify the regression though.

I can easily reproduce this in isolation as well, not just in my project. I followed the instructions to launch the official samples. Then I edited the browser-amd-editor to the follwing:

                        require(['vs/editor/editor.main'], function () {
                                var editor = monaco.editor.create(document.getElementById('container'), {
                                        value: ['function x() {', '\tconsole.log("Hello world!");', '}'].join('\n'),
-                                       language: 'javascript'
+                                       language: 'plaintext'
                                });
+
+                               monaco.editor.setModelLanguage(editor.getModel(), 'text/javascript');
                        });
                </script>
        </body>

There is no syntax highlighting.

Screenshot from 2022-07-22 18-26-52

Then I changed samples/package.json to

                                "file-loader": "^6.2.0",
                                "glob": "^7.2.0",
                                "html-webpack-plugin": "^5.5.0",
-                               "monaco-editor": "^0.32.1",
+                               "monaco-editor": "^0.31.1",
                                "monaco-editor-webpack-plugin": "^7.0.1",
                                "style-loader": "^3.3.1",
                                "terser-webpack-plugin": "^5.3.1",

npm i and restart npm run simpleserver

Screenshot from 2022-07-22 18-26-56

Prinzhorn commented 1 year ago

Thanks for the fix! It's great to see movement again.

roblourens commented 1 year ago

I'm not sure how to verify- is there a way to run the latest code in the monaco playground?

alexdima commented 1 year ago

I'm not sure how to verify- is there a way to run the latest code in the monaco playground?

Unfortunately not yet, @hediet is working on a playground that would allow that. Given this is difficult to verify, I will optimistically mark it verified until our infrastructure in the editor is better.