[Bug] Regression in 0.45.0 - Command Palette does not work if Monaco is wrapped in Angular

[ghost]

Reproducible in vscode.dev or in VS Code Desktop?

• Not reproducible in vscode.dev or VS Code Desktop

Reproducible in the monaco editor playground?

• Not reproducible in the monaco editor playground

Monaco Editor Playground Link

Minimal Example:

https://github.com/FrameworkSystemsGmbH/AngularMonaco

Monaco Editor Playground Code

No response

Reproduction Steps

• Clone https://github.com/FrameworkSystemsGmbH/AngularMonaco
• npm i
• npm run serve
• Open in browser and hit F1

Actual (Problematic) Behavior

If Monaco 0.45.0 is wrapped within an Angular App, the Command Palette is empty.

The console shows this error:

standaloneServices.ts:244 TypeError: T.reduce is not a function
    at w.setElements (quickInputList.ts:688:33)
    at d.update (quickInput.ts:1034:17)
    at d.show (quickInput.ts:315:8)
    at d.show (quickInput.ts:912:9)
    at b.doShowOrPick (quickAccess.ts:145:10)
    at b.show (quickAccess.ts:36:8)
    at u.run (standaloneCommandsQuickAccess.ts:75:48)
    at u.runEditorCommand (editorExtensions.ts:386:15)
    at editorExtensions.ts:320:109
    at editorExtensions.ts:315:11
notify @ standaloneServices.ts:244

Expected Behavior

The Command Palette should work correctly. Latest working version is 0.44.0

Additional Context

I have tried Angular 15.x, 16.x and 17.x to no avail.

Downgrading to Monaco 0.44.0 fixes the issue.

Loading Monaco with loader.js in a plain HTML-File works fine.

[hediet]

Can you reproduce this in the monaco editor playground?

[ghost]

• Playground works fine
• loader.js (AMD) in a single index.html works fine
• loader.js (AMD) with 0.44.0 in NG works
• loader.js (AMD) with 0.45.0 in NG does not work

I know one could say it's an NG problem, but something must have happend to the monaco code that is responsible for the incompatibility with NG since 0.44.0 works perfectly.

[TylerLeonhardt]

We do have a call to .reduce in quickInputList.ts:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/quickInputList.ts#L674

but... I mean that hasn't changed in years. It seems like a reasonable claim that for some reason inputElements isn't a typical array... but why would that be the case?

The only interesting call to setElements is this one:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/quickInput.ts#L1069

The getter & setter for this.items is here:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/quickInput.ts#L599-L615

and this._items starts out as an empty array:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/quickInput.ts#L507

The other side of the coin is if for some reason we fail to get the Commands ...and that somehow sets a non-array type...but that doesn't seem to be the case:

We return an array here:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/editor/contrib/quickAccess/browser/commandsQuickAccess.ts#L48

We return an array here:
https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/commandsQuickAccess.ts#L187

and the only change that went in between 0.44.0 and 0.45.0 is this which wouldn't cause the issue:
microsoft/vscode#201015

Is it possible some mangling could be going on @FSDRE? Can you please provide a minimum repro so that it's easier to debug?

I'd also be curious if other quick picks, like symbols, also fail.

[ghost]

@TylerLeonhardt Mangling is not used. Monaco gets copied 1:1 from node_modules to the assets folder:

{
  "glob": "**/*",
  "input": "node_modules/monaco-editor/min/vs",
  "output": "assets/monaco/min/vs"
},
{
  "glob": "**/*",
  "input": "node_modules/monaco-editor/min-maps/vs",
  "output": "assets/monaco/min-maps/vs"
}

This does not run through any of NG's build processes where minification or mangling is applied.

The LoaderService uses the provided loader.js in order to load Monaco from its original minified sources from the assets folder and provides the HTML element for the monaco.editor.create() method. That's exactly the same call one would do without Angular.

Now here's the weird part:

I checked all your above mentioned links and you are right, I don't see any issues, too. But fact is, if I hit F1 in monaco and set a breakpoint at

this.elements = inputElements.reduce((result, item, index) => {

it gets called twice. On the first call inputElements is an array with length 0. On the second call though, inputElements is a Promise (???). And surely enough promises do not have reduce methods.

Now the question I cannot answer is, where does the Promise come from?

The Promise is even there if I run the LoaderService and monaco.editor.create() outside of the Angular Zone just to make sure there is no moneky-patching going on.

The provided repo is as minimal as it gets. It's the minimal size of a running NG-App.

If you run Chrome with --remote-debugging-port=9222 and hit F5 in VSCode, the debugger should attach.

I am also investing more time into the issue since I need it fixed and I really want to know what's going on. It's a weird one for sure 😄

[ghost]

Ok I got to the culprit of it:

https://github.com/microsoft/vscode/blob/42c45d4b8124d2c6fa11e5b461cf8df30749c332/src/vs/platform/quickinput/browser/pickerQuickAccess.ts#L284

Don't call me insane, but this line gets called even if providedPicks is a Promise.

if I put

providedPicks instanceof Promise

into the browser watch it returns false. The prototype is Promise, though.

This is a honest WTF-moment... what is going on here?

It might be that zone.js could be the issue, don't really know yet.

[ghost]

If I use Monaco 0.44.0, providedPicks is a ZoneAwarePromise and providedPicks instanceof Promise evaluates to true.

As soon as I use 0.45.0, providedPicks is a Promise and providedPicks instanceof Promise evaluates to false.

[TylerLeonhardt]

Wait in 0.45.0 you're saying Promise instance Promise evaluates....... to false?

[ghost]

I am confused and I am honstly not sure if I am understanding this correctly. I made 2 screenshots.

In 0.44.0 providedPicks is shown as ZoneAwarePromise and line 276 does NOT get hit. So !(providedPicks instanceof Promise) evaluates to false. 291 does get hit and awaitedPicks contains all the actions for the command palette.

In 0.45.0 providedPicks is shown as Promise and line 276 gets hit! So !(providedPicks instanceof Promise) evaluates to true resulting in the error with .reduce(). After that line 291 does NOT get hit and the command palette is empty.

What confuses me is that in both cases the watcher shows providedPicks as <not available>. I don't know how to interpret that.

Can you make any sense of that?

[TylerLeonhardt]

Honestly not sure what could be happening here. This is going to be tricky for us to track down without a simple repro. I can't say that we'll have the time to look deep into this... but if you find a solution and the PR is small we would for sure consider taking it. I'm very suspicious of some hijacking of the global Promise which is causing this.

[ghost]

What is your definition of a simple repo? Since you need the Angular wrapper, the given one is as simple as one could provide it. I cannot really make it any smaller.

I understand that this issue may not be of your concern and I am positive that Angular somehow monkey-patches the global Promise in order to achieve its zone-aware change detection. It's not a Monaco issue per se.

With the knowledge about the ZoneAwarePromise, I will try to post this in the Angular repo. I myself am not able to pin down the actual root cause of the behavior.

Nevertheless, thanks for your input and the maintainance of the editor!

[FSDKO]

The problem occurres exactly since v0.45.0-dev-20231125.
The essential commit is this: microsoft/vscode@59787d6

Since zone.js has a big (still unsolved) problem with native async/await (angular/angular#31730) this change causes the Problem.

[hediet]

The essential commit is this: microsoft/vscode@59787d6

We cannot revert that because of the angular zone library having issues with native promises.

I'm sorry that it broke for you, but this is yet another proof that we can only investigate issues that reproduce in the monaco editor playground.

[FSDKO]

Thats fine for me.
But now we know, where the issue comes from and we have to deal without Angular 😢.