fix: ensure removed extraLibs are not used in tsWorker #4544
Conversation
|
@microsoft-github-policy-service agree |
|
@aiday-mar @hediet is there anything else I can provide to help this get reviewed? |
src/language/typescript/tsWorker.ts
Outdated
| if (uriAsString.startsWith('ts:extralib-')) { | ||
| return false; | ||
| } | ||
| // Otherwise, the prefix will be file:/// and the name will be what the user provided to add/setExtraLibs |
There was a problem hiding this comment.
I don't think the prefix will always be file://. I think you can pass any URI (as a string) to addExtraLib, including with a different scheme, or with file as a scheme, but with a hostname (i.e. file://some-host/some/path.
There was a problem hiding this comment.
thanks for the catch @tomprince , sorry it took so long for me to revisit this.
I've made changes in 6505802 to address your comment
jcarrus
force-pushed
the
justin.extra-libs
branch
from
54e5c40
to
6505802
Compare
|
An updated video that shows me walking through the code and each of the reproductions after adding 6505802 commit. 2024-09-27.19-18-05.mp4@hediet and @aiday-mar , is there any chance you could take a look at this PR? I've just rebased/etc. |
This PR fixes #4023, #3580, and #3465 by adding a check to the
getScriptFileNames()function of the tsWorker to ensure that no "extraLibs" are double returned or returned after being removed.The source of all three of these bugs is that sometimes when an extraLib is loaded, a Model is created. This model is then improperly included in the result of
getScriptFileNames.getScriptFileNames, once because it's open as a model, and once because it's an extraLib.getScriptFileNamesbecause a model has been opened.There seems to be a workaround in userspace (#3465 (comment)) where a consumer of the monaco editor can intentionally open a model for the extra lib and then close it manually after removing the extra lib, but that doesn't seem like the expected usage.
Note that there is some randomness to the following reproductions. There seems to be some criteria that monaco uses to determine when a model should be opened that I can't entirely understand. Watch the video for an example of the reproductions.
2024-06-01.11-54-39.mp4
Explanation of fix for #4023
A simplified reproduction of the issue can be found here: https://microsoft.github.io/monaco-editor/playground.html?sourceLanguages=http%3A%2F%2Flocalhost%3A5002%2Fout%2Flanguages%2Famd-tsc#XQAAAAIuAwAAAAAAAABBqQkHQ5Njdzms4V_4h8Ymy5YEB9krg8_BDE676yYR7b49CMZv7S9qVO95gb8Ved019aXivCXcGVmGp8AmBO4Tl4DbmQrjS-_8JUSzTnmGkQF66fpyv4IoHfFYPv_-NeZTt0oM2r1te1xRtKktg77q900SsxXOxrd0MhhVQgrGDTVmFm5U38foXKhFjQO2UR0619gf8hxom2y_wTeKNSRPXBL2A_7Ayiq-pDvs_vFBOf3ubonYDlkvYgpvAMIIe4tsam_BQ3EQ7ujSqjHjndU5goroaQuRxfquFWWKJzLjCPWAdesCx8TaOdljIq8QuSA5cd-PddCrnkJunNTPiP1HACroC8d9TGVZAfNDwtkL1XYf4A0n4tzPzrtEddsavsIG7lg62q4Cc3az-zOzTyGJk20jiLcEtxYZLpunJEFZGOAVfo5z4B5tVV6nq2NInMCYva7XxQSGgycII-2VPW7PD4MqLEMR_dLqRky7_sD2wFjq01T6x1yJyCAICiQe4Nw1JssFKjI2pudJvyrI-dnUL9jqOJvGyp8KLvev4bmWXCgnehldKa3PxtRC8xsrR1PZ4UGMnMTzWmvkPWXedf_NdBWs
Clicking "one", then "two", then "three" will reliably result in an end state where the "one" extralib is ignored and the "two" is used.
2024-06-01.11-57-46.mp4
Explanation of fix for #3580
A simplified reproduction is available here: https://microsoft.github.io/monaco-editor/playground.html#XQAAAAJwAQAAAAAAAABBqQkHQ5NjdMjwa-jY7SIQ9S7DNlzs5W-mwj0fe1ZCDRFc9ws9XQE0SJE1jc2VKxhaLFIw9vEWSxW3yscwzeXY_bpo6KbnrhSaY2QSt9k-SLd-nFCTuOim633hTWmvFSR0kpdcf0rVGFgSFg_JQb86ebJtjgrlcmp2kozW3cBgg08HTNQwVEkv3PKCAcT4kNbbhq0D7Uy9QLpnClziyz9w-y9VKaYhYR_kT6AFAmtzC7yqVgMSpLVH4v_uWNp_le-SCQJez5QpwyM-a0JOQAPZFm-DWYXG0e2N7qUO2nicqUFrvx6ew7oHnHvTurei-Wq6xVPAQCn0ZPRr8fbQN2y3qrfbh_-8EXIA
and two definitions are shown for the
trimfunction because a model is open in addition to the extra lib.2024-06-01.12-00-04.mp4
Explanation of fix for #3465
A simplified reproduction can be found here: https://microsoft.github.io/monaco-editor/playground.html#XQAAAALHAgAAAAAAAABBqQkHQ5NjdMjwa-jY7SIQ9S7DNlzs5W-mwj0fe1ZCDRFc9ws9XQE0SJE1jc2VKxhaLFIw9vEWSxW3yscw2XE-SBm5Kf01uPmdfsotI5HnROq3VvYNxqUMtR6J5SjpnTSLKSSepKpvVNibSX6o07rgCc3W9x-SGcmhjSXxodTyIUbNHY1z5KiG8jaq2V94UPqUNN7WMFM6_c2HlapK7XLyOoCXC_uKKIDRDpYypcD-U0rTD9yJXkrQxM8Pfnr67zIQY3MZaERjO4soaPi3DN7PC9gM6uJnkGdJD50jFiTaPLiVPz2W8xd96GUexFBnv77th_wJWpDSprZptOVPq9aYwFlJVAyK7W3WZidDuTVzt1WHCXsUh58xn3-hjW_kTp-htqE52oPiU-sHQ4vDUr838uQf4QeE9-GzS9XNysr9-zDb64F8H7EBS61DyPG6KsoCNywkyXjBNV8DqKwMq7JSQfLJ5Oo9Axu3EfzHtgpuZzp-zELM7giRWZUR_GVKFw
However, this reproduction does not reliably create a model for me. If the user forces a model to be created by "peeking", then the reproduction fails reliably (see video for an example).
2024-06-01.12-01-33.mp4