TypeScript 3.0 language service isn't working properly in IE 11 (in Monaco Editor)

[kpreisser]

TypeScript Version: 3.0.1-insiders.20180726

Search Terms: IE11, language service, monaco editor

Code

Open the TypeScript Playground (Monaco Editor using the language service of current TypeScript version 3.0.1-insiders.20180726) in IE 11 (on Windows 10, 8.1 or 7) and enter the following code:

// In IE 11, this doesn't show an error:
let f = function () {
    let g = function() {
        Math.xxxxxxxyyyyzzzz(aaaaaa, bbbbbb, ccccc);
    };
};

// This correctly shows an error:
Math.xxxxxxxyyyyzzzz(aaaaaa, bbbbbb, ccccc);

Expected behavior:
The unknown properties and variables should be shown as error (which works in other browsers like Edge, Firefox, Chrome):

Actual behavior:
For the inner function, unknown properties/variables are not shown as error in IE 11:

With the previous playground version (I think 2.9.x), this worked correctly. I don't know what could cause this behavior in IE 11, since the console also doesn't show any errors.

Additionally, if you enable noUnusedLocals in the Monaco Editor, the editor will sometimes show variables as unused even though they are used (I currently don't have a screenshot of that).

Unfortunately, for a software product at work we still need to use IE 11 (as WebBrowser control in a .NET app), so it would be nice if TypeScript with Monaco Editor would still work properly on IE 11.

Playground Link: http://www.typescriptlang.org/play/#src=%2F%2F%20In%20IE%2011%2C%20this%20doesn't%20show%20an%20error%3A%0D%0Alet%20f%20%3D%20function%20()%20%7B%0D%0A%20%20%20%20let%20g%20%3D%20function()%20%7B%0D%0A%20%20%20%20%20%20%20%20Math.xxxxxxxyyyyzzzz(aaaaaa%2C%20bbbbbb%2C%20ccccc)%3B%0D%0A%20%20%20%20%7D%3B%0D%0A%7D%3B%0D%0A%0D%0A%2F%2F%20This%20correctly%20shows%20an%20error%3A%0D%0AMath.xxxxxxxyyyyzzzz(aaaaaa%2C%20bbbbbb%2C%20ccccc)%3B%0D%0A%0D%0A

Thank you!

[kpreisser]

Bisecting shows that this problem started to appear with commit 9044589 (#24508). When I revert this commit on top of the current master branch (32e99ba), the language service starts to work again in IE 11.

The commit changed the usage of an array to a Map that is created with createMap<T>. However, in IE 11, this doesn't use the native Map implementation, but one created by shimMap(). When I change the MapCtr to always use shimMap(), the problem appears also in non-IE browsers, which suggests there is some issue with that map implementation.

However, I have not yet investigated what difference betwen the shimMap and a regular Map could cause this issue.

[kpreisser]

It seems the code in checkDeferredNodes() (checker.ts) relies on the implementation of Map.forEach() to visit values that are added while forEach() is executing. This behavor is documented for a native Map:

New values added before forEach has finished will be visited.

However, the shim map uses a regular for-in loop, for which this behavior is not guaranteed (and the new values not being visited seem to cause this issue):

Properties added to the object over which iteration is occurring may either be visited or omitted from iteration.

Is it expected that the map is modified while forEach is running? In that case I think the shim map (which is used on IE 11) needs to be adjusted to support this behavior.

Thanks!

[kpreisser]

Hi,

I have created a commit (kpreisser@aa34bbf) to fix this issue.

My idea was to create a keys array on the map when the first iterator or forEach call starts. When new values are added to the map while iterators are active, they will be appended to that array, and when values are removed from the map, they will also be removed from the array and the iterator's indexes will be adjusted.

This fixes the issue with IE 11. However, the fix has a drawback because the map contains references to all active iterators (to be able to adjust their indexes when keys are removed).

• This means if iterators are thrown away before they are finished, this may create a memory leak because the iterator references will never be removed from the map.
• Also, if keys are removed from the map after iterators are thrown away before they are finished, these delete operations will be slow since they need to iterate over the keys array used by iterators.

If it is expected that the code can create iterators multiple times and throw them away before they are finished, this could be solved e.g. by only allowing one iterator at a time to be active (so that previously created iterators will then throw when their .next() method is called).

An alternative fix for this issue might be to always use a native map in checkDeferredNodes(), as that specific code doesn't need an iterator. However, this would mean that the code wouldn't work on ES5 environments anymore that don't have a native Map, and there might still be other code that also creates a Map with createMap() and depends on the behavior of forEach/iterators to visit values that are added while forEach/iterators are running.

Edit: My preferred solution would be to simply not support delete() operations while iterators are active, as I think that is an uncommon operation (i.e. you would still be able to delete values while iterators are active, but this would clear the keys array, so the iterator would throw an error the next time you call .next()). This would solve the memory leak problem and the slow delete() operation.

What do you think? Do you have other ideas how to fix this issue?
Should I create a PR?

Thanks!

[DanielRosenwasser]

Should be fixed. Sorry about that!