search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.08k stars 29.24k forks source link

invalid tail call #85863

Closed ctf0 closed 4 years ago

ctf0 commented 4 years ago

Env

Version: 1.41.0-insider
Commit: e15ff0097ceb84c4e28a67dac41c2d8dcd4cf027
Date: 2019-11-29T09:05:24.307Z
Electron: 6.1.5
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Darwin x64 18.7.0

Steps to Reproduce:

1- create an untitled view or open a file 2- using the below commands in order workbench.action.moveEditorToxxxGroup > workbench.action.editorLayoutSingle > workbench.action.moveEditorToxxxGroup will

xxx == any of "right / left / below / above"

ERR this.root.style is not a function: TypeError: this.root.style is not a function
    at v.style (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:523:1007)
    at p.style (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:532:100)
    at e.updateStyles (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:5689:1001)
    at e.create (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:2743:368)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:5742:600
    at Array.forEach (<anonymous>)
    at B.renderWorkbench (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:5742:507)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:5739:356
    at h.invokeFunction (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:1547:994)
    at B.startup (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:5739:94)
    at Q.open (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:6295:216)

Does this issue occur when all extensions are disabled?: Yes

bpasero commented 4 years ago

I can reproduce. Exact steps:

=> 🐛 at this point the grid is broken. reloading the window does not restore the grid anymore failing with [3]

I cannot reproduce in Stable.

[1]

ERR Cannot read property 'length' of undefined: TypeError: Cannot read property 'length' of undefined
    at GridView.trySet2x2 (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/grid/gridview.js:807:36)
    at SerializableGrid.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/grid/grid.js:437:31)
    at GridWidgetView.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:43:33)
    at CenteredViewLayout.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/centered/centeredViewLayout.js:63:27)
    at EditorPart.doLayout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:725:39)
    at EditorPart.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:720:18)
    at LeafNode.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/grid/gridview.js:448:23)
    at VerticalViewItem.layout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/splitview/splitview.js:75:23)
    at SplitView.layoutViews (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/splitview/splitview.js:627:26)
    at SplitView.relayout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/splitview/splitview.js:534:18)
    at SplitView.onViewChange (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/splitview/splitview.js:403:22)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/splitview/splitview.js:461:70
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/common/event.js:70:138
    at Emitter.fire (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/common/event.js:458:38)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/common/event.js:43:93
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/common/event.js:70:138
    at Emitter.fire (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/common/event.js:458:38)
    at EditorPart.doSetGridWidget (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:704:37)
    at EditorPart.doCreateGridControlWithState (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:695:18)
    at EditorPart.applyLayout (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:303:18)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorCommands.js:184:32
    at InstantiationService.invokeFunction (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/platform/instantiation/common/instantiationService.js:43:27)
    at CommandService._tryExecuteCommand (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/services/commands/common/commandService.js:73:74)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/services/commands/common/commandService.js:63:47
    at async Object.triggerAndDisposeAction (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/common/actions.js:83:17)
    at async file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/common/actions.js:62:21

[2]

Error: Invalid tail call
    at Object.tail2 (arrays.js:19)
    at GridView.addView (gridview.js:556)
    at SerializableGrid._addView (grid.js:219)
    at SerializableGrid.addView (grid.js:199)
    at EditorPart.doAddGroup (editorPart.js:353)
    at EditorPart.addGroup (editorPart.js:344)
    at moveActiveEditorToGroup (editorCommands.js:142)
    at moveActiveEditor (editorCommands.js:91)
    at handler (editorCommands.js:51)
    at idOrCommand.handler (commands.js:34)
    at InstantiationService.invokeFunction (instantiationService.js:43)
    at CommandService._tryExecuteCommand (commandService.js:73)
    at commandService.js:63
    at async Object.triggerAndDisposeAction (actions.js:83)

[3]

ERR this.root.style is not a function: TypeError: this.root.style is not a function
    at GridView.style (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/grid/gridview.js:539:23)
    at SerializableGrid.style (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/base/browser/ui/grid/grid.js:166:27)
    at EditorPart.updateStyles (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/parts/editor/editorPart.js:586:29)
    at EditorPart.create (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/part.js:44:18)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/workbench.js:225:34
    at Array.forEach (<anonymous>)
    at Workbench.renderWorkbench (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/workbench.js:223:15)
    at file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/workbench.js:75:26
    at InstantiationService.invokeFunction (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/platform/instantiation/common/instantiationService.js:43:27)
    at Workbench.startup (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/browser/workbench.js:62:38)
    at DesktopMain.open (file:///Users/bpasero/Development/Microsoft/monaco/out/vs/workbench/electron-browser/desktop.main.js:61:52)
sbatten commented 4 years ago

My investigation shows that this was introduced when fixing https://github.com/microsoft/vscode/issues/84425 with commit https://github.com/microsoft/vscode/commit/1bd3750d655b6f2da429307419092af01a311653.

This change unintentionally broke an assumption that has always been made with the grid view: The root node will always be a branch node. I first tried updating grid view to allow for this behavior but the changes were more dangerous as that assumption permeates the many classes of gridview.

Thus, the above commit simply allows the rootNode to be single node group. I've also added an exception to be thrown in case someone is already in this bad state so that the editor part will be forced to create a new grid.

@joaomoreno please check my work

bpasero commented 4 years ago

Thanks 🍺

joaomoreno commented 4 years ago

@sbatten Great fix! :clap: Those "edge" cases. :laughing: