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!

[DanielRosenwasser]

fixe

[kpreisser]

Hi @DanielRosenwasser,

can you give me a reference of how the issue is being fixed? The problem still occurs for me when using the current master branch (1e55d65), and the code of shimMap() or checkDeferredNodes() doesn't seem to have changed.

Note that this issue only occurs when running the TypeScript Services in IE 11, e.g. using the Monaco Editor.

Thanks!

[kpreisser]

Hi @DanielRosenwasser,
this issue is still present on IE 11 using current master branch (54a5be1).
Can you reopen it?

Please note that this issue is not the same as #26068; this one is caused due to the shimMap used on IE 11 behaving differently than a regular Map when using .forEach().

Thank you!

[kpreisser]

Hi @DanielRosenwasser,
this issue is still present on IE 11 using current master branch (799656a).
Can you reopen it?

Thank you!

[DanielRosenwasser]

@kpreisser you're saying it's still present on master - but does #28921 not fix the issue? If you have an idea of the issue, can you give a more explicit example of what's not working?

[kpreisser]

Hi @DanielRosenwasser,

I tested this issue by compiling TypeScript from commit 799656a, then using the Monaco Editor (0.15.6) and replacing the typescriptServices.js with the built version, and then opening the Monaco Editor in IE 11 and entering the code shown in the first comment.
The issue was still present on IE 11, so it doesn't seem to be fixed. However when I build TS with the changes from #27292, it works correctly.

I think #28921 is unrelated to this issue because this issue is about the Map that is created by createMap<T>, as described in my comments above:
On IE 11, that method returns a custom Map implementation (shimMap), but it has a difference to the native Map implementation: When iterating with forEach(), the shimMap does not visit values that are added during iteration (because it uses a regular for-in loop), whereas for a native Map this behavior is documented.

You can check this by adding the following code at the end of compiler/core.ts:

    let myMap = createMap<string>();
    myMap.set("1", "Original Value");

    myMap.forEach((value, key) => {
        console.log(`myMap: ${key}->${value}`);

        // Add a new value - the map should provide this one in the next iteration.
        if (key == "1")
            myMap.set("0", "New Value");
    });

Then, if you compile it and run it in IE 11, the console will show:

myMap: 1->Original Value

However, running it in any other browser (that supports a native Map with iterators) it will log:

myMap: 1->Original Value
myMap: 0->New Value

Now, because commit 9044589 changed the use of an array to a Map created with createMap<Node> and then iterating through it with deferredNodes!.forEach(...), it exhibits that problem because the code seems to add values to the Map while iterating it with .forEach(). (However, I have not checked exactly what that code is doing).

The result is what you can see in the screenshot of my first comment - the unkonwn variables in the inner function are not shown as errors when running the Monaco Editor/Playground in IE 11.
However, the Monaco Editor still supports IE 11, so I think the TS Language Service should also support it.

In PR #27292, I changed the implementation of shimMap to ensure values added during iteration are visited in the forEach function, which seems to fix the issue.

Thanks!

[kpreisser]

This bug is now fixed as #27292 has been merged.