From be69c1c891c2ea81cfd6a28fabb20e05e305d4df Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 3 Oct 2019 13:26:26 -0700 Subject: [PATCH 01/15] First idea - load cells all at once. Change updates to not update full cell Change global properties to be per cell Change cells to check for updates before rendering --- package-lock.json | 4 +- .../history-react/interactiveCell.tsx | 17 ++- .../history-react/interactivePanel.tsx | 2 - .../interactive-common/mainState.ts | 10 +- .../interactive-common/mainStateController.ts | 143 +++++++++++------- .../native-editor/nativeCell.tsx | 26 ++-- .../native-editor/nativeEditor.tsx | 10 +- .../nativeEditorStateController.ts | 4 +- .../react-common/monacoEditor.tsx | 2 +- 9 files changed, 131 insertions(+), 87 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6b4425355528..bbdda43446c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14145,7 +14145,7 @@ }, "os-locale": { "version": "3.1.0", - "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-3.1.0.tgz", + "resolved": false, "integrity": "sha512-Z8l3R4wYWM40/52Z+S265okfFj8Kt2cC2MKY+xNi3kFs+XGI7WXu/I309QQQYbRW4ijiZ+yxs9pqEhJh0DqW3Q==", "dev": true, "requires": { @@ -14162,7 +14162,7 @@ }, "resolve-from": { "version": "4.0.0", - "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-4.0.0.tgz", + "resolved": false, "integrity": "sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==", "dev": true }, diff --git a/src/datascience-ui/history-react/interactiveCell.tsx b/src/datascience-ui/history-react/interactiveCell.tsx index f00a13214649..aecd092e0e17 100644 --- a/src/datascience-ui/history-react/interactiveCell.tsx +++ b/src/datascience-ui/history-react/interactiveCell.tsx @@ -4,6 +4,7 @@ import '../../client/common/extensions'; import { nbformat } from '@jupyterlab/coreutils'; +import * as fastDeepEqual from 'fast-deep-equal'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; import * as React from 'react'; @@ -35,10 +36,6 @@ interface IInteractiveCellProps { editorOptions?: monacoEditor.editor.IEditorOptions; editExecutionCount?: string; editorMeasureClassName?: string; - selectedCell?: string; - focusedCell?: string; - hideOutput?: boolean; - showLineNumbers?: boolean; onCodeChange(changes: monacoEditor.editor.IModelContentChange[], cellId: string, modelId: string): void; onCodeCreated(code: string, file: string, cellId: string, modelId: string): void; openLink(uri: monacoEditor.Uri): void; @@ -71,11 +68,15 @@ export class InteractiveCell extends React.Component { } public componentDidUpdate(prevProps: IInteractiveCellProps) { - if (this.props.selectedCell === this.props.cellVM.cell.id && prevProps.selectedCell !== this.props.selectedCell) { - this.giveFocus(this.props.focusedCell === this.props.cellVM.cell.id); + if (this.props.cellVM.selected && !prevProps.cellVM.selected) { + this.giveFocus(this.props.cellVM.focused); } } + public shouldComponentUpdate(nextProps: IInteractiveCellProps): boolean { + return !fastDeepEqual(this.props, nextProps); + } + public scrollAndFlash() { if (this.wrapperRef && this.wrapperRef.current) { this.wrapperRef.current.scrollIntoView({ behavior: 'auto', block: 'nearest', inline: 'nearest' }); @@ -210,7 +211,7 @@ export class InteractiveCell extends React.Component { focused={this.onCodeFocused} unfocused={this.onCodeUnfocused} keyDown={this.props.keyDown} - showLineNumbers={this.props.showLineNumbers} + showLineNumbers={this.props.cellVM.showLineNumbers} /> ); } @@ -238,7 +239,7 @@ export class InteractiveCell extends React.Component { } private shouldRenderResults(): boolean { - return this.isCodeCell() && this.hasOutput() && this.getCodeCell().outputs && this.getCodeCell().outputs.length > 0 && !this.props.hideOutput; + return this.isCodeCell() && this.hasOutput() && this.getCodeCell().outputs && this.getCodeCell().outputs.length > 0 && !this.props.cellVM.hideOutput; } private onCellKeyDown = (event: React.KeyboardEvent) => { diff --git a/src/datascience-ui/history-react/interactivePanel.tsx b/src/datascience-ui/history-react/interactivePanel.tsx index 0d978809a73d..2505354db1c8 100644 --- a/src/datascience-ui/history-react/interactivePanel.tsx +++ b/src/datascience-ui/history-react/interactivePanel.tsx @@ -344,8 +344,6 @@ export class InteractivePanel extends React.Component ); diff --git a/src/datascience-ui/interactive-common/mainState.ts b/src/datascience-ui/interactive-common/mainState.ts index 4afcab242481..161dbfd0db87 100644 --- a/src/datascience-ui/interactive-common/mainState.ts +++ b/src/datascience-ui/interactive-common/mainState.ts @@ -26,6 +26,8 @@ export interface ICellViewModel { showLineNumbers?: boolean; hideOutput?: boolean; useQuickEdit?: boolean; + selected: boolean; + focused: boolean; inputBlockToggled(id: string): void; } @@ -135,7 +137,9 @@ export function createEditableCellVM(executionCount: number): ICellViewModel { inputBlockShow: true, inputBlockText: '', inputBlockCollapseNeeded: false, - inputBlockToggled: noop + inputBlockToggled: noop, + selected: false, + focused: false }; } @@ -176,7 +180,9 @@ export function createCellVM(inputCell: ICell, settings: IDataScienceSettings | inputBlockShow: true, inputBlockText: inputText, inputBlockCollapseNeeded: (inputLinesCount > 1), - inputBlockToggled: inputBlockToggled + inputBlockToggled: inputBlockToggled, + selected: false, + focused: false }; } diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index a5d3258e2f83..6e8a0e240226 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. 'use strict'; import * as fastDeepEqual from 'fast-deep-equal'; +import * as immutable from 'immutable'; import { min } from 'lodash'; // tslint:disable-next-line: no-require-imports import cloneDeep = require('lodash/cloneDeep'); @@ -31,7 +32,14 @@ import { getSettings, updateSettings } from '../react-common/settingsReactSide'; import { detectBaseTheme } from '../react-common/themeDetector'; import { InputHistory } from './inputHistory'; import { IntellisenseProvider } from './intellisenseProvider'; -import { createCellVM, createEditableCellVM, extractInputText, generateTestState, ICellViewModel, IMainState } from './mainState'; +import { + createCellVM, + createEditableCellVM, + extractInputText, + generateTestState, + ICellViewModel, + IMainState +} from './mainState'; import { initializeTokenizer, registerMonacoLanguage } from './tokenizer'; export interface IMainStateControllerProps { @@ -284,7 +292,7 @@ export class MainStateController implements IMessageHandler { // Update our state this.setState({ - cellVMs: this.state.cellVMs.filter(c => c.cell.id !== cellId), + cellVMs: [...this.state.cellVMs.filter(c => c.cell.id !== cellId)], undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), skipNextScroll: true }); @@ -366,9 +374,7 @@ export class MainStateController implements IMessageHandler { public clearAllOutputs = () => { const newList = this.state.cellVMs.map(cellVM => { - const newVM = cloneDeep(cellVM); - newVM.cell.data.outputs = []; - return newVM; + return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); }); this.setState({ cellVMs: newList @@ -473,26 +479,60 @@ export class MainStateController implements IMessageHandler { public codeLostFocus = (cellId: string) => { this.onCodeLostFocus(cellId); if (this.state.focusedCell === cellId) { + const newVMs = [...this.state.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(cellId); + if (oldSelect >= 0) { + newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { focused: false }); + } // Only unfocus if we haven't switched somewhere else yet - this.setState({ focusedCell: undefined }); + this.setState({ focusedCell: undefined, cellVMs: newVMs }); } } public codeGotFocus = (cellId: string | undefined) => { - this.setState({ selectedCell: cellId, focusedCell: cellId }); + // Skip if already has focus + if (cellId !== this.state.focusedCell) { + const newVMs = [...this.state.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(this.state.selectedCell); + if (oldSelect >= 0) { + newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { selected: false, focused: false }); + } + const newSelect = this.findCellIndex(cellId); + if (newSelect >= 0) { + newVMs[newSelect] = immutable.merge(newVMs[newSelect], { selected: true, focused: true }); + } + + // Save the whole thing in our state. + this.setState({ selectedCell: cellId, focusedCell: cellId, cellVMs: newVMs }); + } } public selectCell = (cellId: string, focusedCell?: string) => { - this.setState({ selectedCell: cellId, focusedCell }); + // Skip if already the same cell + if (this.state.selectedCell !== cellId) { + const newVMs = [...this.state.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(this.state.selectedCell); + if (oldSelect >= 0) { + newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { selected: false, focused: false }); + } + const newSelect = this.findCellIndex(cellId); + if (newSelect >= 0) { + newVMs[newSelect] = immutable.merge(newVMs[newSelect], { selected: true, focused: focusedCell === newVMs[newSelect].cell.id }); + } + + // Save the whole thing in our state. + this.setState({ selectedCell: cellId, focusedCell, cellVMs: newVMs }); + } } public changeCellType = (cellId: string, newType: 'code' | 'markdown') => { const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0 && this.state.cellVMs[index].cell.data.cell_type !== newType) { - const newVM = cloneDeep(this.state.cellVMs[index]); - newVM.cell.data.cell_type = newType; const cellVMs = [...this.state.cellVMs]; - cellVMs.splice(index, 1, newVM); + cellVMs[index] = immutable.updateIn(this.state.cellVMs[index], ['cell', 'data', 'cell_type'], () => newType); this.setState({ cellVMs }); } } @@ -506,7 +546,7 @@ export class MainStateController implements IMessageHandler { // This should be from our last entry. Switch this entry to read only, and add a new item to our list if (inputCell && inputCell.cell.id === Identifiers.EditCellId) { - let newCell = cloneDeep(inputCell); + let newCell = immutable.mergeDeep(inputCell); // Change this editable cell to not editable. newCell.cell.state = CellState.executing; @@ -593,6 +633,10 @@ export class MainStateController implements IMessageHandler { return nonEdit; } + public findCellIndex(cellId?: string): number { + return this.state.cellVMs.findIndex(cvm => cvm.cell.id === cellId); + } + public getMonacoId(cellId: string): string | undefined { return this.cellIdToMonacoId.get(cellId); } @@ -601,8 +645,7 @@ export class MainStateController implements IMessageHandler { const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); - newVMs[index].showLineNumbers = !newVMs[index].showLineNumbers; + newVMs[index] = immutable.merge(newVMs[index], { showLineNumbers: !newVMs[index].showLineNumbers }); this.setState({ cellVMs: newVMs }); } } @@ -611,8 +654,7 @@ export class MainStateController implements IMessageHandler { const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); - newVMs[index].hideOutput = !newVMs[index].hideOutput; + newVMs[index] = immutable.merge(newVMs[index], { hideOutput: !newVMs[index].hideOutput }); this.setState({ cellVMs: newVMs }); } } @@ -726,37 +768,38 @@ export class MainStateController implements IMessageHandler { this.insertCell(cell); } - protected insertCell(cell: ICell, position?: number, isMonaco?: boolean): ICellViewModel | undefined { - if (cell) { - const showInputs = getSettings().showCellInputCode; - const collapseInputs = getSettings().collapseCellInputCodeByDefault; - let cellVM: ICellViewModel = createCellVM(cell, getSettings(), this.inputBlockToggled, this.props.defaultEditable); + protected prepareCellVM(cell: ICell, isMonaco?: boolean): ICellViewModel { + const showInputs = getSettings().showCellInputCode; + const collapseInputs = getSettings().collapseCellInputCodeByDefault; + let cellVM: ICellViewModel = createCellVM(cell, getSettings(), this.inputBlockToggled, this.props.defaultEditable); - // Set initial cell visibility and collapse - cellVM = this.alterCellVM(cellVM, showInputs, !collapseInputs); + // Set initial cell visibility and collapse + cellVM = this.alterCellVM(cellVM, showInputs, !collapseInputs); - if (cellVM) { - if (isMonaco) { - cellVM.useQuickEdit = false; - } + if (isMonaco) { + cellVM.useQuickEdit = false; + } - const newList = [...this.state.cellVMs]; - // Make sure to use the same array so our entire state doesn't update - if (position !== undefined && position >= 0) { - newList.splice(position, 0, cellVM); - } else { - newList.push(cellVM); - } - this.setState({ - cellVMs: newList, - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), - redoStack: this.state.redoStack, - skipNextScroll: false - }); + return cellVM; + } - return cellVM; - } + protected insertCell(cell: ICell, position?: number, isMonaco?: boolean): ICellViewModel { + const cellVM = this.prepareCellVM(cell, isMonaco); + const newList = [...this.state.cellVMs]; + // Make sure to use the same array so our entire state doesn't update + if (position !== undefined && position >= 0) { + newList.splice(position, 0, cellVM); + } else { + newList.push(cellVM); } + this.setState({ + cellVMs: newList, + undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), + redoStack: this.state.redoStack, + skipNextScroll: false + }); + + return cellVM; } protected suspendUpdates() { @@ -822,12 +865,12 @@ export class MainStateController implements IMessageHandler { // Turn off updates so we generate all of the cell vms without rendering. this.suspendUpdates(); - // Update all of the vms + // Generate all of the VMs const cells = payload.cells as ICell[]; - cells.forEach(c => this.finishCell(c)); + const vms = cells.map(c => this.prepareCellVM(c, true)); // Set our state to not being busy anymore. Clear undo stack as this can't be undone. - this.setState({ busy: false, loadTotal: payload.cells.length, undoStack: [] }); + this.setState({ busy: false, loadTotal: payload.cells.length, undoStack: [], cellVMs: vms }); // Turn updates back on and resend the state. this.resumeUpdates(); @@ -845,8 +888,7 @@ export class MainStateController implements IMessageHandler { if (executingCells && executingCells.length) { const newVMs = [...this.state.cellVMs]; executingCells.forEach(s => { - newVMs[s.i] = cloneDeep(s.cvm); - newVMs[s.i].cell.state = CellState.finished; + newVMs[s.i] = immutable.updateIn(s.cvm, ['cell', 'state'], () => CellState.finished); }); this.setState({ cellVMs: newVMs }); } @@ -1012,16 +1054,15 @@ export class MainStateController implements IMessageHandler { // Have to make a copy of the cell VM array or // we won't actually update. const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); // Check to see if our code still matches for the cell (in liveshare it might be updated from the other side) if (concatMultilineString(newVMs[index].cell.data.source) !== concatMultilineString(cell.data.source)) { const newText = extractInputText(cell, getSettings()); - newVMs[index].inputBlockText = newText; + newVMs[index] = { ...newVMs[index], cell: cell, inputBlockText: newText }; + } else { + newVMs[index] = { ...newVMs[index], cell: cell }; } - newVMs[index].cell = cell; - this.setState({ cellVMs: newVMs, currentExecutionCount: newExecutionCount diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index e6c08965480e..2654a0d5c4b8 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -4,6 +4,7 @@ import '../../client/common/extensions'; import { nbformat } from '@jupyterlab/coreutils'; +import * as fastDeepEqual from 'fast-deep-equal'; import * as React from 'react'; import { concatMultilineString } from '../../client/datascience/common'; @@ -32,10 +33,6 @@ interface INativeCellProps { maxTextSize?: number; stateController: NativeEditorStateController; monacoTheme: string | undefined; - hideOutput?: boolean; - showLineNumbers?: boolean; - selectedCell?: string; - focusedCell?: string; focusCell(cellId: string, focusCode: boolean): void; selectCell(cellId: string): void; } @@ -56,6 +53,7 @@ export class NativeCell extends React.Component; } else { @@ -64,14 +62,18 @@ export class NativeCell extends React.Component { - return this.props.selectedCell === this.cellId; + return this.props.cellVM.selected; } private isFocused = () => { - return this.props.focusedCell === this.cellId; + return this.props.cellVM.focused; } private renderNormalCell() { @@ -220,7 +222,7 @@ export class NativeCell extends React.Component ); } @@ -726,10 +728,10 @@ export class NativeCell extends React.Component { let classes = 'collapse-bar'; - if (this.props.selectedCell === this.props.cellVM.cell.id && this.props.focusedCell !== this.props.cellVM.cell.id) { + if (this.isSelected() && !this.isFocused()) { classes += ' collapse-bar-selected'; } - if (this.props.focusedCell === this.props.cellVM.cell.id) { + if (this.isFocused()) { classes += ' collapse-bar-focused'; } diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 253bb11bab41..e0901716bde2 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -3,6 +3,7 @@ 'use strict'; import './nativeEditor.less'; +import * as immutable from 'immutable'; import * as React from 'react'; import { noop } from '../../client/common/utils/misc'; @@ -24,8 +25,6 @@ import { NativeEditorStateController } from './nativeEditorStateController'; // tslint:disable: react-this-binding-issue // tslint:disable-next-line:no-require-imports no-var-requires const debounce = require('lodash/debounce') as typeof import('lodash/debounce'); -// tslint:disable-next-line: no-require-imports -import cloneDeep = require('lodash/cloneDeep'); interface INativeEditorProps { skipDefault: boolean; @@ -292,8 +291,7 @@ export class NativeEditor extends React.Component diff --git a/src/datascience-ui/native-editor/nativeEditorStateController.ts b/src/datascience-ui/native-editor/nativeEditorStateController.ts index 94c685b9d182..0663b3e41912 100644 --- a/src/datascience-ui/native-editor/nativeEditorStateController.ts +++ b/src/datascience-ui/native-editor/nativeEditorStateController.ts @@ -112,7 +112,9 @@ export class NativeEditorStateController extends MainStateController { inputBlockShow: true, inputBlockText: '', inputBlockCollapseNeeded: false, - inputBlockToggled: noop + inputBlockToggled: noop, + selected: cells[0].selected, + focused: cells[0].focused }; this.setState({ cellVMs: [newVM], undoStack: this.pushStack(this.getState().undoStack, cells) }); } else { diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 6117fde784d7..25a8bed93f73 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -384,7 +384,7 @@ export class MonacoEditor extends React.Component Date: Thu, 3 Oct 2019 13:57:28 -0700 Subject: [PATCH 02/15] Fix delete to not select at the same time --- .../interactive-common/mainStateController.ts | 23 ++++++++++++++--- .../native-editor/nativeCell.tsx | 25 ++++++++----------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index 6e8a0e240226..1c56ff2802f6 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -285,14 +285,29 @@ export class MainStateController implements IMessageHandler { } public deleteCell = (cellId: string) => { - const cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); - if (cellVM) { + const index = this.findCellIndex(cellId); + if (index) { this.sendMessage(InteractiveWindowMessages.DeleteCell); - this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellVM.cell.id }); + this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellId }); + + // Recompute select/focus if this item has either + let newSelection = this.state.selectedCell; + let newFocused = this.state.focusedCell; + const newVMs = [...this.state.cellVMs.filter(c => c.cell.id !== cellId)]; + const nextOrPrev = index === this.state.cellVMs.length - 1 ? index - 1 : index; + if (this.state.selectedCell === cellId || this.state.focusedCell === cellId) { + if (nextOrPrev >= 0) { + newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: this.state.focusedCell === cellId }; + newSelection = newVMs[nextOrPrev].cell.id; + newFocused = newVMs[nextOrPrev].focused ? newVMs[nextOrPrev].cell.id : undefined; + } + } // Update our state this.setState({ - cellVMs: [...this.state.cellVMs.filter(c => c.cell.id !== cellId)], + cellVMs: newVMs, + selectedCell: newSelection, + focusedCell: newFocused, undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), skipNextScroll: true }); diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 2654a0d5c4b8..a035128f1490 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -186,11 +186,16 @@ export class NativeCell extends React.Component) => { - // When we receive a click, propagate upwards. Might change our state - ev.stopPropagation(); - this.lastKeyPressed = undefined; - const focusedCell = this.isFocused() ? this.cellId : undefined; - this.props.stateController.selectCell(this.cellId, focusedCell); + if (ev.nativeEvent.target) { + const elem = ev.nativeEvent.target as HTMLElement; + if (!elem.className.includes('image')) { + // Not a click on an image, select the cell. + ev.stopPropagation(); + this.lastKeyPressed = undefined; + const focusedCell = this.isFocused() ? this.cellId : undefined; + this.props.stateController.selectCell(this.cellId, focusedCell); + } + } } private onMouseDoubleClick = (ev: React.MouseEvent) => { @@ -293,12 +298,8 @@ export class NativeCell extends React.Component { const cellId = this.props.cellVM.cell.id; const deleteCell = () => { - const cellToSelect = this.getNextCellId() || this.getPrevCellId(); this.props.stateController.possiblyDeleteCell(cellId); this.props.stateController.sendCommand(NativeCommandType.DeleteCell, 'mouse'); - setTimeout(() => { - if (cellToSelect) { - this.moveSelection(cellToSelect); - } - }, 10); }; const runAbove = () => { this.props.stateController.runAbove(cellId); From 9534968772efade50c599131584d7a5e284c61dd Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 3 Oct 2019 15:07:35 -0700 Subject: [PATCH 03/15] Make a pending state and a rendered state to use for rendering Some perf fixes for monaco editor. Skip laying out parent etc until necessary --- .../interactive-common/mainStateController.ts | 229 +++++++++--------- .../react-common/monacoEditor.tsx | 20 +- 2 files changed, 131 insertions(+), 118 deletions(-) diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index 1c56ff2802f6..5ad09e6f40e9 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -57,7 +57,8 @@ export interface IMainStateControllerProps { // tslint:disable-next-line: max-func-body-length export class MainStateController implements IMessageHandler { private stackLimit = 10; - private state: IMainState; + private pendingState: IMainState; + private renderedState: IMainState; private postOffice: PostOffice = new PostOffice(); private intellisenseProvider: IntellisenseProvider; private onigasmPromise: Deferred | undefined; @@ -68,7 +69,7 @@ export class MainStateController implements IMessageHandler { // tslint:disable-next-line:max-func-body-length constructor(private props: IMainStateControllerProps) { - this.state = { + this.renderedState = { editorOptions: this.computeEditorOptions(), cellVMs: [], busy: true, @@ -89,7 +90,7 @@ export class MainStateController implements IMessageHandler { // Add test state if necessary if (!this.props.skipDefault) { - this.state = generateTestState(this.inputBlockToggled, '', this.props.defaultEditable); + this.renderedState = generateTestState(this.inputBlockToggled, '', this.props.defaultEditable); } // Setup the completion provider for monaco. We only need one @@ -99,7 +100,7 @@ export class MainStateController implements IMessageHandler { if (this.props.skipDefault) { if (this.props.testMode) { // Running a test, skip the tokenizer. We want the UI to display synchronously - this.state = { tokenizerLoaded: true, ...this.state }; + this.renderedState = { tokenizerLoaded: true, ...this.renderedState }; // However we still need to register python as a language registerMonacoLanguage(); @@ -108,6 +109,9 @@ export class MainStateController implements IMessageHandler { } } + // Copy the rendered state + this.pendingState = { ...this.renderedState }; + // Add ourselves as a handler for the post office this.postOffice.addHandler(this); @@ -251,16 +255,16 @@ export class MainStateController implements IMessageHandler { } public stopBusy = () => { - if (this.state.busy) { + if (this.pendingState.busy) { this.setState({ busy: false }); } } public redo = () => { // Pop one off of our redo stack and update our undo - const cells = this.state.redoStack[this.state.redoStack.length - 1]; - const redoStack = this.state.redoStack.slice(0, this.state.redoStack.length - 1); - const undoStack = this.pushStack(this.state.undoStack, this.state.cellVMs); + const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1]; + const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1); + const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs); this.sendMessage(InteractiveWindowMessages.Redo); this.setState({ cellVMs: cells, @@ -272,9 +276,9 @@ export class MainStateController implements IMessageHandler { public undo = () => { // Pop one off of our undo stack and update our redo - const cells = this.state.undoStack[this.state.undoStack.length - 1]; - const undoStack = this.state.undoStack.slice(0, this.state.undoStack.length - 1); - const redoStack = this.pushStack(this.state.redoStack, this.state.cellVMs); + const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1]; + const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1); + const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs); this.sendMessage(InteractiveWindowMessages.Undo); this.setState({ cellVMs: cells, @@ -286,18 +290,18 @@ export class MainStateController implements IMessageHandler { public deleteCell = (cellId: string) => { const index = this.findCellIndex(cellId); - if (index) { + if (index >= 0) { this.sendMessage(InteractiveWindowMessages.DeleteCell); this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellId }); // Recompute select/focus if this item has either - let newSelection = this.state.selectedCell; - let newFocused = this.state.focusedCell; - const newVMs = [...this.state.cellVMs.filter(c => c.cell.id !== cellId)]; - const nextOrPrev = index === this.state.cellVMs.length - 1 ? index - 1 : index; - if (this.state.selectedCell === cellId || this.state.focusedCell === cellId) { + let newSelection = this.pendingState.selectedCell; + let newFocused = this.pendingState.focusedCell; + const newVMs = [...this.pendingState.cellVMs.filter(c => c.cell.id !== cellId)]; + const nextOrPrev = index === this.pendingState.cellVMs.length - 1 ? index - 1 : index; + if (this.pendingState.selectedCell === cellId || this.pendingState.focusedCell === cellId) { if (nextOrPrev >= 0) { - newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: this.state.focusedCell === cellId }; + newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: this.pendingState.focusedCell === cellId }; newSelection = newVMs[nextOrPrev].cell.id; newFocused = newVMs[nextOrPrev].focused ? newVMs[nextOrPrev].cell.id : undefined; } @@ -308,7 +312,7 @@ export class MainStateController implements IMessageHandler { cellVMs: newVMs, selectedCell: newSelection, focusedCell: newFocused, - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), skipNextScroll: true }); } @@ -345,7 +349,7 @@ export class MainStateController implements IMessageHandler { public save = () => { // We have to take the current value of each cell to make sure we have the correct text. - this.state.cellVMs.forEach(c => this.updateCellSource(c.cell.id)); + this.pendingState.cellVMs.forEach(c => this.updateCellSource(c.cell.id)); // Then send the save with the new state. this.sendMessage(InteractiveWindowMessages.SaveAll, { cells: this.getNonEditCellVMs().map(cvm => cvm.cell) }); @@ -376,11 +380,11 @@ export class MainStateController implements IMessageHandler { } public canRedo = () => { - return this.state.redoStack.length > 0; + return this.pendingState.redoStack.length > 0; } public canUndo = () => { - return this.state.undoStack.length > 0; + return this.pendingState.undoStack.length > 0; } public canClearAllOutputs = () => { @@ -388,7 +392,7 @@ export class MainStateController implements IMessageHandler { } public clearAllOutputs = () => { - const newList = this.state.cellVMs.map(cellVM => { + const newList = this.pendingState.cellVMs.map(cellVM => { return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); }); this.setState({ @@ -398,7 +402,7 @@ export class MainStateController implements IMessageHandler { public gotoCellCode = (cellId: string) => { // Find our cell - const cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); + const cellVM = this.pendingState.cellVMs.find(c => c.cell.id === cellId); // Send a message to the other side to jump to a particular cell if (cellVM) { @@ -408,9 +412,9 @@ export class MainStateController implements IMessageHandler { public copyCellCode = (cellId: string) => { // Find our cell. This is also supported on the edit cell - let cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); - if (!cellVM && this.state.editCellVM && cellId === this.state.editCellVM.cell.id) { - cellVM = this.state.editCellVM; + let cellVM = this.pendingState.cellVMs.find(c => c.cell.id === cellId); + if (!cellVM && this.pendingState.editCellVM && cellId === this.pendingState.editCellVM.cell.id) { + cellVM = this.pendingState.editCellVM; } // Send a message to the other side to jump to a particular cell @@ -437,19 +441,19 @@ export class MainStateController implements IMessageHandler { public export = () => { // Send a message to the other side to export our current list - const cellContents: ICell[] = this.state.cellVMs.map((cellVM: ICellViewModel, _index: number) => { return cellVM.cell; }); + const cellContents: ICell[] = this.pendingState.cellVMs.map((cellVM: ICellViewModel, _index: number) => { return cellVM.cell; }); this.sendMessage(InteractiveWindowMessages.Export, cellContents); } // When the variable explorer wants to refresh state (say if it was expanded) public refreshVariables = (newExecutionCount?: number) => { - this.sendMessage(InteractiveWindowMessages.GetVariablesRequest, newExecutionCount === undefined ? this.state.currentExecutionCount : newExecutionCount); + this.sendMessage(InteractiveWindowMessages.GetVariablesRequest, newExecutionCount === undefined ? this.pendingState.currentExecutionCount : newExecutionCount); } public toggleVariableExplorer = () => { - this.sendMessage(InteractiveWindowMessages.VariableExplorerToggle, !this.state.variablesVisible); - this.setState({ variablesVisible: !this.state.variablesVisible }); - if (!this.state.variablesVisible) { + this.sendMessage(InteractiveWindowMessages.VariableExplorerToggle, !this.pendingState.variablesVisible); + this.setState({ variablesVisible: !this.pendingState.variablesVisible }); + if (!this.pendingState.variablesVisible) { this.refreshVariables(); } } @@ -469,7 +473,7 @@ export class MainStateController implements IMessageHandler { } public readOnlyCodeCreated = (_text: string, file: string, id: string, monacoId: string) => { - const cell = this.state.cellVMs.find(c => c.cell.id === id); + const cell = this.pendingState.cellVMs.find(c => c.cell.id === id); if (cell) { // Pass this onto the completion provider running in the extension this.sendMessage(InteractiveWindowMessages.AddCell, { @@ -493,12 +497,12 @@ export class MainStateController implements IMessageHandler { public codeLostFocus = (cellId: string) => { this.onCodeLostFocus(cellId); - if (this.state.focusedCell === cellId) { - const newVMs = [...this.state.cellVMs]; + if (this.pendingState.focusedCell === cellId) { + const newVMs = [...this.pendingState.cellVMs]; // Switch the old vm const oldSelect = this.findCellIndex(cellId); if (oldSelect >= 0) { - newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { focused: false }); + newVMs[oldSelect] = { ...newVMs[oldSelect], focused: false }; } // Only unfocus if we haven't switched somewhere else yet this.setState({ focusedCell: undefined, cellVMs: newVMs }); @@ -507,16 +511,16 @@ export class MainStateController implements IMessageHandler { public codeGotFocus = (cellId: string | undefined) => { // Skip if already has focus - if (cellId !== this.state.focusedCell) { - const newVMs = [...this.state.cellVMs]; + if (cellId !== this.pendingState.focusedCell) { + const newVMs = [...this.pendingState.cellVMs]; // Switch the old vm - const oldSelect = this.findCellIndex(this.state.selectedCell); + const oldSelect = this.findCellIndex(this.pendingState.selectedCell); if (oldSelect >= 0) { - newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { selected: false, focused: false }); + newVMs[oldSelect] = { ...newVMs[oldSelect], selected: false, focused: false }; } const newSelect = this.findCellIndex(cellId); if (newSelect >= 0) { - newVMs[newSelect] = immutable.merge(newVMs[newSelect], { selected: true, focused: true }); + newVMs[newSelect] = { ...newVMs[newSelect], selected: true, focused: true }; } // Save the whole thing in our state. @@ -526,16 +530,16 @@ export class MainStateController implements IMessageHandler { public selectCell = (cellId: string, focusedCell?: string) => { // Skip if already the same cell - if (this.state.selectedCell !== cellId) { - const newVMs = [...this.state.cellVMs]; + if (this.pendingState.selectedCell !== cellId) { + const newVMs = [...this.pendingState.cellVMs]; // Switch the old vm - const oldSelect = this.findCellIndex(this.state.selectedCell); + const oldSelect = this.findCellIndex(this.pendingState.selectedCell); if (oldSelect >= 0) { - newVMs[oldSelect] = immutable.merge(newVMs[oldSelect], { selected: false, focused: false }); + newVMs[oldSelect] = { ...newVMs[oldSelect], selected: false, focused: false }; } const newSelect = this.findCellIndex(cellId); if (newSelect >= 0) { - newVMs[newSelect] = immutable.merge(newVMs[newSelect], { selected: true, focused: focusedCell === newVMs[newSelect].cell.id }); + newVMs[newSelect] = { ...newVMs[newSelect], selected: true, focused: focusedCell === newVMs[newSelect].cell.id }; } // Save the whole thing in our state. @@ -544,10 +548,10 @@ export class MainStateController implements IMessageHandler { } public changeCellType = (cellId: string, newType: 'code' | 'markdown') => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); - if (index >= 0 && this.state.cellVMs[index].cell.data.cell_type !== newType) { - const cellVMs = [...this.state.cellVMs]; - cellVMs[index] = immutable.updateIn(this.state.cellVMs[index], ['cell', 'data', 'cell_type'], () => newType); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); + if (index >= 0 && this.pendingState.cellVMs[index].cell.data.cell_type !== newType) { + const cellVMs = [...this.pendingState.cellVMs]; + cellVMs[index] = immutable.updateIn(this.pendingState.cellVMs[index], ['cell', 'data', 'cell_type'], () => newType); this.setState({ cellVMs }); } } @@ -561,7 +565,7 @@ export class MainStateController implements IMessageHandler { // This should be from our last entry. Switch this entry to read only, and add a new item to our list if (inputCell && inputCell.cell.id === Identifiers.EditCellId) { - let newCell = immutable.mergeDeep(inputCell); + let newCell = cloneDeep(inputCell); // Change this editable cell to not editable. newCell.cell.state = CellState.executing; @@ -603,9 +607,9 @@ export class MainStateController implements IMessageHandler { // Stick in a new cell at the bottom that's editable and update our state // so that the last cell becomes busy this.setState({ - cellVMs: [...this.state.cellVMs, newCell], - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), - redoStack: this.state.redoStack, + cellVMs: [...this.pendingState.cellVMs, newCell], + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), + redoStack: this.pendingState.redoStack, skipNextScroll: false, submittedText: true }); @@ -615,41 +619,41 @@ export class MainStateController implements IMessageHandler { this.sendMessage(InteractiveWindowMessages.SubmitNewCell, { code, id: newCell.cell.id }); } } else if (inputCell.cell.data.cell_type === 'code') { - // Update our input cell to be in progress again - inputCell.cell.state = CellState.executing; - - // Clear our outputs - inputCell.cell.data.outputs = []; - - // Update our state to display the new status - this.setState({ - cellVMs: [...this.state.cellVMs] - }); + const index = this.findCellIndex(inputCell.cell.id); + if (index >= 0) { + // Update our input cell to be in progress again and clear outputs + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = { ...inputCell, cell: { ...inputCell.cell, state: CellState.executing, data: { ...inputCell.cell.data, outputs: [] } } }; + this.setState({ + cellVMs: newVMs + }); + } // Send a message to rexecute this code this.sendMessage(InteractiveWindowMessages.ReExecuteCell, { code, id: inputCell.cell.id }); } else if (inputCell.cell.data.cell_type === 'markdown') { - // Change the input on the cell - inputCell.cell.data.source = code; - inputCell.inputBlockText = code; - - // Update our state to display the new status - this.setState({ - cellVMs: [...this.state.cellVMs] - }); + const index = this.findCellIndex(inputCell.cell.id); + if (index >= 0) { + // Change the input on the cell + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = { ...inputCell, inputBlockText: code, cell: { ...inputCell.cell, data: { ...inputCell.cell.data, source: code } } }; + this.setState({ + cellVMs: newVMs + }); + } } } public findCell(cellId?: string): ICellViewModel | undefined { - const nonEdit = this.state.cellVMs.find(cvm => cvm.cell.id === cellId); + const nonEdit = this.pendingState.cellVMs.find(cvm => cvm.cell.id === cellId); if (!nonEdit && cellId === Identifiers.EditCellId) { - return this.state.editCellVM; + return this.pendingState.editCellVM; } return nonEdit; } public findCellIndex(cellId?: string): number { - return this.state.cellVMs.findIndex(cvm => cvm.cell.id === cellId); + return this.pendingState.cellVMs.findIndex(cvm => cvm.cell.id === cellId); } public getMonacoId(cellId: string): string | undefined { @@ -657,34 +661,37 @@ export class MainStateController implements IMessageHandler { } public toggleLineNumbers = (cellId: string) => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { - const newVMs = [...this.state.cellVMs]; + const newVMs = [...this.pendingState.cellVMs]; newVMs[index] = immutable.merge(newVMs[index], { showLineNumbers: !newVMs[index].showLineNumbers }); this.setState({ cellVMs: newVMs }); } } public toggleOutput = (cellId: string) => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { - const newVMs = [...this.state.cellVMs]; + const newVMs = [...this.pendingState.cellVMs]; newVMs[index] = immutable.merge(newVMs[index], { hideOutput: !newVMs[index].hideOutput }); this.setState({ cellVMs: newVMs }); } } public setState(newState: {}, callback?: () => void) { + // Add to writable state (it should always reflect the current conditions) + this.pendingState = { ...this.pendingState, ...newState }; + if (this.suspendUpdateCount > 0) { // Just save our new state - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; if (callback) { callback(); } } else { // Send a UI update this.props.setState(newState, () => { - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; if (callback) { callback(); } @@ -693,7 +700,7 @@ export class MainStateController implements IMessageHandler { } public renderUpdate(newState: {}) { - const oldCount = this.state.pendingVariableCount; + const oldCount = this.renderedState.pendingVariableCount; // This method should be called during the render stage of anything // using this state Controller. That's because after shouldComponentUpdate @@ -701,7 +708,7 @@ export class MainStateController implements IMessageHandler { // See https://reactjs.org/docs/react-component.html // Otherwise we set the state in the callback during setState and this can be // too late for any render code to use the stateController. - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; // If the new state includes any cellVM changes, send an update to the other side if ('cellVMs' in newState) { @@ -709,13 +716,13 @@ export class MainStateController implements IMessageHandler { } // If the new state includes pendingVariableCount and it's gone to zero, send a message - if (this.state.pendingVariableCount === 0 && oldCount !== 0) { + if (this.renderedState.pendingVariableCount === 0 && oldCount !== 0) { setTimeout(() => this.sendMessage(InteractiveWindowMessages.VariablesComplete), 1); } } public getState(): IMainState { - return this.state; + return this.pendingState; } // Adjust the visibility or collapsed state of a cell @@ -800,7 +807,7 @@ export class MainStateController implements IMessageHandler { protected insertCell(cell: ICell, position?: number, isMonaco?: boolean): ICellViewModel { const cellVM = this.prepareCellVM(cell, isMonaco); - const newList = [...this.state.cellVMs]; + const newList = [...this.pendingState.cellVMs]; // Make sure to use the same array so our entire state doesn't update if (position !== undefined && position >= 0) { newList.splice(position, 0, cellVM); @@ -809,8 +816,8 @@ export class MainStateController implements IMessageHandler { } this.setState({ cellVMs: newList, - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), - redoStack: this.state.redoStack, + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), + redoStack: this.pendingState.redoStack, skipNextScroll: false }); @@ -825,7 +832,7 @@ export class MainStateController implements IMessageHandler { if (this.suspendUpdateCount > 0) { this.suspendUpdateCount -= 1; if (this.suspendUpdateCount === 0) { - this.setState(this.state); // This should cause an update + this.setState(this.pendingState); // This should cause an update } } @@ -896,12 +903,12 @@ export class MainStateController implements IMessageHandler { this.suspendUpdates(); // When we restart, make sure to turn off all executing cells. They aren't executing anymore - const executingCells = this.state.cellVMs + const executingCells = this.pendingState.cellVMs .map((cvm, i) => { return { cvm, i }; }) .filter(s => s.cvm.cell.state !== CellState.error && s.cvm.cell.state !== CellState.finished); if (executingCells && executingCells.length) { - const newVMs = [...this.state.cellVMs]; + const newVMs = [...this.pendingState.cellVMs]; executingCells.forEach(s => { newVMs[s.i] = immutable.updateIn(s.cvm, ['cell', 'state'], () => CellState.finished); }); @@ -955,7 +962,7 @@ export class MainStateController implements IMessageHandler { // Update theme if necessary const newSettings = JSON.parse(payload as string); const dsSettings = newSettings as IDataScienceExtraSettings; - if (dsSettings && dsSettings.extraSettings && dsSettings.extraSettings.theme !== this.state.theme) { + if (dsSettings && dsSettings.extraSettings && dsSettings.extraSettings.theme !== this.pendingState.theme) { // User changed the current theme. Rerender this.postOffice.sendUnsafeMessage(CssMessages.GetCssRequest, { isDark: this.computeKnownDark() }); this.postOffice.sendUnsafeMessage(CssMessages.GetMonacoThemeRequest, { isDark: this.computeKnownDark() }); @@ -969,7 +976,7 @@ export class MainStateController implements IMessageHandler { private getAllCells = () => { // Send all of our cells back to the other side - const cells = this.state.cellVMs.map((cellVM: ICellViewModel) => { + const cells = this.pendingState.cellVMs.map((cellVM: ICellViewModel) => { return cellVM.cell; }); @@ -977,14 +984,14 @@ export class MainStateController implements IMessageHandler { } private getNonEditCellVMs(): ICellViewModel[] { - return this.state.cellVMs; + return this.pendingState.cellVMs; } private clearAllSilent = () => { // Update our state this.setState({ cellVMs: [], - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), skipNextScroll: true, busy: false // No more progress on delete all }); @@ -992,7 +999,7 @@ export class MainStateController implements IMessageHandler { private inputBlockToggled = (id: string) => { // Create a shallow copy of the array, let not const as this is the shallow array copy that we will be changing - const cellVMArray: ICellViewModel[] = [...this.state.cellVMs]; + const cellVMArray: ICellViewModel[] = [...this.pendingState.cellVMs]; const cellVMIndex = cellVMArray.findIndex((value: ICellViewModel) => { return value.cell.id === id; }); @@ -1028,7 +1035,7 @@ export class MainStateController implements IMessageHandler { } private alterAllCellVMs = (visible: boolean, expanded: boolean) => { - const newCells = this.state.cellVMs.map((value: ICellViewModel) => { + const newCells = this.pendingState.cellVMs.map((value: ICellViewModel) => { return this.alterCellVM(value, visible, expanded); }); @@ -1042,14 +1049,14 @@ export class MainStateController implements IMessageHandler { const info: IInteractiveWindowInfo = { visibleCells: this.getNonEditCellVMs().map(cvm => cvm.cell), cellCount: this.getNonEditCellVMs().length, - undoCount: this.state.undoStack.length, - redoCount: this.state.redoStack.length + undoCount: this.pendingState.undoStack.length, + redoCount: this.pendingState.redoStack.length }; this.sendMessage(InteractiveWindowMessages.SendInfo, info); } private updateOrAdd = (cell: ICell, allowAdd?: boolean) => { - const index = this.state.cellVMs.findIndex((c: ICellViewModel) => { + const index = this.pendingState.cellVMs.findIndex((c: ICellViewModel) => { return c.cell.id === cell.id && c.cell.line === cell.line && c.cell.file === cell.file; @@ -1058,9 +1065,9 @@ export class MainStateController implements IMessageHandler { // This means the cell existed already so it was actual executed code. // Use its execution count to update our execution count. const newExecutionCount = cell.data.execution_count ? - Math.max(this.state.currentExecutionCount, parseInt(cell.data.execution_count.toString(), 10)) : - this.state.currentExecutionCount; - if (newExecutionCount !== this.state.currentExecutionCount && this.state.variablesVisible) { + Math.max(this.pendingState.currentExecutionCount, parseInt(cell.data.execution_count.toString(), 10)) : + this.pendingState.currentExecutionCount; + if (newExecutionCount !== this.pendingState.currentExecutionCount && this.pendingState.variablesVisible) { // We also need to update our variable explorer when the execution count changes // Use the ref here to maintain var explorer independence this.refreshVariables(newExecutionCount); @@ -1068,10 +1075,10 @@ export class MainStateController implements IMessageHandler { // Have to make a copy of the cell VM array or // we won't actually update. - const newVMs = [...this.state.cellVMs]; + const newVMs = [...this.pendingState.cellVMs]; // Check to see if our code still matches for the cell (in liveshare it might be updated from the other side) - if (concatMultilineString(newVMs[index].cell.data.source) !== concatMultilineString(cell.data.source)) { + if (concatMultilineString(this.pendingState.cellVMs[index].cell.data.source) !== concatMultilineString(cell.data.source)) { const newText = extractInputText(cell, getSettings()); newVMs[index] = { ...newVMs[index], cell: cell, inputBlockText: newText }; } else { @@ -1135,14 +1142,14 @@ export class MainStateController implements IMessageHandler { const variable = payload as IJupyterVariable; // Only send the updated variable data if we are on the same execution count as when we requested it - if (variable && variable.executionCount !== undefined && variable.executionCount === this.state.currentExecutionCount) { - const stateVariable = this.state.variables.findIndex(v => v.name === variable.name); + if (variable && variable.executionCount !== undefined && variable.executionCount === this.pendingState.currentExecutionCount) { + const stateVariable = this.pendingState.variables.findIndex(v => v.name === variable.name); if (stateVariable >= 0) { - const newState = [...this.state.variables]; + const newState = [...this.pendingState.variables]; newState.splice(stateVariable, 1, variable); this.setState({ variables: newState, - pendingVariableCount: Math.max(0, this.state.pendingVariableCount - 1) + pendingVariableCount: Math.max(0, this.pendingState.pendingVariableCount - 1) }); } } @@ -1156,7 +1163,7 @@ export class MainStateController implements IMessageHandler { const variablesResponse = payload as IJupyterVariablesResponse; // Check to see if we have moved to a new execution count only send our update if we are on the same count as the request - if (variablesResponse.executionCount === this.state.currentExecutionCount) { + if (variablesResponse.executionCount === this.pendingState.currentExecutionCount) { this.setState({ variables: variablesResponse.variables, pendingVariableCount: variablesResponse.variables.length @@ -1222,7 +1229,7 @@ export class MainStateController implements IMessageHandler { // We also get this in our response, but computing is more reliable // than searching for it. - if (this.state.knownDark !== computedKnownDark) { + if (this.pendingState.knownDark !== computedKnownDark) { this.darkChanged(computedKnownDark); } diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 25a8bed93f73..151f5d70e2f5 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -38,6 +38,7 @@ interface IMonacoEditorState { model: monacoEditor.editor.ITextModel | null; visibleLineCount: number; attached: boolean; + parentUpdated: boolean; } // Need this to prevent wiping of the current value on a componentUpdate. react-monaco-editor has that problem. @@ -61,7 +62,7 @@ export class MonacoEditor extends React.Component(); this.measureWidthRef = React.createRef(); this.debouncedUpdateEditorSize = debounce(this.updateEditorSize.bind(this), 150); @@ -156,6 +157,12 @@ export class MonacoEditor extends React.Component { + this.throttledUpdateWidgetPosition(); + this.updateWidgetParent(editor); + })); + // Update our margin to include the correct line number style this.updateMargin(editor); @@ -309,6 +316,7 @@ export class MonacoEditor extends React.Component { this.throttledUpdateWidgetPosition(); + this.updateWidgetParent(this.state.editor); } private updateBackgroundStyle = () => { @@ -382,9 +390,6 @@ export class MonacoEditor extends React.Component