fix: ensure removed extraLibs are not used in tsWorker

[jcarrus]

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.

• If the extra lib has been added, but not removed, then it is (sometimes) returned twice from getScriptFileNames, once because it's open as a model, and once because it's an extraLib.
• If the extra lib has been added and then subsequently removed, the lib is still returned from getScriptFileNames because 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 trim function 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

[jcarrus]

@microsoft-github-policy-service agree

[jcarrus]

@aiday-mar @hediet is there anything else I can provide to help this get reviewed?

[tomprince]

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.

[jcarrus]

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]

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.

[jcarrus]

@hediet and @aiday-mar sorry to bother you two again. Should I be waiting for someone from the Microsoft team to review this or should I be trying to get more community approvals? or something else?

I don't see anything that describes the PR etiquette here in CONTRIBUTING.md.

I'm also open to hearing that this isn't the right way to address this, but would appreciate some kind of indication.

[aiday-mar]

Hi @jcarrus thank you for the ping. The PR needs to be reviewed by a community member. Unfortunately I am not the owner of this area so I can not review it.