search

CodinGame / monaco-vscode-api

VSCode public API plugged on the monaco editor
MIT License
214 stars 29 forks source link

Version 2.1.x broke vscode-oniguruma in dev mode #344

Closed kevinvalk closed 5 months ago

kevinvalk commented 5 months ago

When upgrading to 2.1.1 (from 2.0.3) the editor stopped loading properly (in dev mode). Upon further inspection the path onig.wasm was loaded from was wrong. In 2.0.3 it was http://localhost:8000/@fs/node_modules/vscode-oniguruma/release/onig.wasm while in 2.1.1 it has become http://localhost:8000/full/path/of/the/current/url/node_modules.asar.unpacked/vscode-oniguruma/release/onig.wasm.

errors.js:10 Uncaught Error: Failed to execute 'compile' on 'WebAssembly': Incorrect response MIME type. Expected 'application/wasm'.

I am assuming that this error caused the editor to not properly load and when fixed all is well. That said, I am not completely sure on how to debug this further to figure out what the problem is.

CGNonofr commented 5 months ago

Do you have the issue with the demo here? I don't

Can you please provide a reproduction repo? I'm not able to reproduce it in a vite project nor in big webpack projects

CGNonofr commented 5 months ago

I would say it's an inconsistent package versions. It can only happens if the Textmate service doesn't have access to a properly initialized environment and product services

kevinvalk commented 5 months ago

Check, thanks for pointers, going to try to hunt this down or/and provide reproduction repo.

Keep you posted!

kevinvalk commented 5 months ago

Found it, VSCode changed the detection algorithm in 1.86 for detecting if VSCode is running in web or not https://github.com/microsoft/vscode/commit/c52d8fb97acd3852b30a97e1fb7da00bb9f83c7c

In practice this means that if process is an object, VSCode now will assume it is NOT running in web. I guess it is not really the responsibility of this library to take care of this but maybe it makes sense to patch the isWeb flag to always to be true. I do not see how this library can ever be used outside web context?

CGNonofr commented 5 months ago

Yes I was struggling with it as well,especially to run jest tests.

My final solution was to check that process is not defined and throw an error otherwise. There is no reason in 2024 and since webpack 5 for process to be defined in a web env anyway

Some user are using that library inside electron apps so it's probably not a so good idea to hack it

Why is process defined in your case?

kevinvalk commented 5 months ago

Great point!

I thought it was a good idea to augment the process global variable with (vite) defines so I could do things like process.define.ABC to access them. It clearly wasn't that great of an idea as indeed process really should not be defined in web env.

So I refactored some code and the reliance on process is gone :)

Thanks for all of your work once again!

CGNonofr commented 5 months ago

It's worth a troubleshooting section though!

dkattan commented 5 months ago

It's worth a troubleshooting section though!

Yes! I spent hours digging into this until I realized my Vite process.env variables were the culprit!