From e5d338ea647ec195b135ae0abb69373fee6945db Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Tue, 8 Sep 2020 22:37:35 +0200 Subject: [PATCH 1/8] Debounce updating the status indicator of devtools --- .../Preview/DevTools/Tests/index.tsx | 55 +++++---- .../app/components/Preview/DevTools/index.tsx | 109 +++++++++++++----- 2 files changed, 107 insertions(+), 57 deletions(-) diff --git a/packages/app/src/app/components/Preview/DevTools/Tests/index.tsx b/packages/app/src/app/components/Preview/DevTools/Tests/index.tsx index 77323bafdf5..03d7d3af522 100644 --- a/packages/app/src/app/components/Preview/DevTools/Tests/index.tsx +++ b/packages/app/src/app/components/Preview/DevTools/Tests/index.tsx @@ -186,7 +186,12 @@ class Tests extends React.Component { * Every setState call will have to go through this, otherwise we get race conditions * where the underlying state has changed, but the draftState didn't change. */ - setStateDebounced = (setStateFunc, time = 200) => { + setStateDebounced = ( + setStateFunc: + | Partial + | ((state: State, props: DevToolProps) => Partial), + time = 200 + ) => { const draftState = this.draftState || this.state; const newState = @@ -234,7 +239,7 @@ class Tests extends React.Component { selectFile = (file: File) => { this.setStateDebounced( - state => ({ + (state) => ({ selectedFilePath: file.fileName === state.selectedFilePath ? null : file.fileName, }), @@ -244,8 +249,8 @@ class Tests extends React.Component { toggleFileExpansion = (file: File) => { this.setStateDebounced( - oldState => - immer(oldState, state => { + (oldState) => + immer(oldState, (state) => { state.fileExpansionState[file.fileName] = !state.fileExpansionState[ file.fileName ]; @@ -312,10 +317,10 @@ class Tests extends React.Component { const files = Object.keys(this.state.files); const failingTests = files.filter( - f => this.getStatus(this.state.files[f]) === 'fail' + (f) => this.getStatus(this.state.files[f]) === 'fail' ).length; const passingTests = files.filter( - f => this.getStatus(this.state.files[f]) === 'pass' + (f) => this.getStatus(this.state.files[f]) === 'pass' ).length; if (this.props.updateStatus) { @@ -333,8 +338,8 @@ class Tests extends React.Component { } case messages.ADD_FILE: { - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { state.files[data.path] = { tests: {}, fileName: data.path, @@ -346,8 +351,8 @@ class Tests extends React.Component { break; } case 'remove_file': { - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { if (state.files[data.path]) { delete state.files[data.path]; } @@ -358,8 +363,8 @@ class Tests extends React.Component { break; } case messages.FILE_ERROR: { - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { if (state.files[data.path]) { state.files[data.path].fileError = data.error; } @@ -378,8 +383,8 @@ class Tests extends React.Component { case messages.ADD_TEST: { const testName = [...this.currentDescribeBlocks, data.testName]; - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { if (!state.files[data.path]) { state.files[data.path] = { tests: {}, @@ -404,8 +409,8 @@ class Tests extends React.Component { const { test } = data; const testName = [...test.blocks, test.name]; - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { if (!state.files[test.path]) { state.files[test.path] = { tests: {}, @@ -435,8 +440,8 @@ class Tests extends React.Component { const { test } = data; const testName = [...test.blocks, test.name]; - this.setStateDebounced(oldState => - immer(oldState, state => { + this.setStateDebounced((oldState) => + immer(oldState, (state) => { if (!state.files[test.path]) { return; } @@ -524,7 +529,7 @@ class Tests extends React.Component { }; toggleWatching = () => { - this.setStateDebounced(state => ({ watching: !state.watching }), 0); + this.setStateDebounced((state) => ({ watching: !state.watching }), 0); dispatch({ type: 'set-test-watching', watching: !this.state.watching, @@ -540,8 +545,8 @@ class Tests extends React.Component { runTests = (file: File) => { this.setStateDebounced( - oldState => - immer(oldState, state => { + (oldState) => + immer(oldState, (state) => { if (state.files[file.fileName]) { state.files[file.fileName].tests = {}; } @@ -567,15 +572,15 @@ class Tests extends React.Component { const selectedFile = this.state.files[selectedFilePath || '']; const fileStatuses = {}; - Object.keys(this.state.files).forEach(path => { + Object.keys(this.state.files).forEach((path) => { fileStatuses[path] = this.getStatus(this.state.files[path]); }); const tests = []; - Object.keys(this.state.files).forEach(path => { + Object.keys(this.state.files).forEach((path) => { const file = this.state.files[path]; if (file && file.tests) { - Object.keys(file.tests).forEach(t => { + Object.keys(file.tests).forEach((t) => { tests.push(file.tests[t]); }); } @@ -600,7 +605,7 @@ class Tests extends React.Component {
{Object.keys(this.state.files) .sort() - .map(fileName => ( + .map((fileName) => ( { + draftState: State | null = null; constructor(props: Props) { super(props); @@ -139,6 +140,49 @@ export class DevTools extends React.PureComponent { }; } + setStateTimer = null; + /** + * We can call setState 100s of times per second, which puts great strain + * on rendering from React. We debounce the rendering so that we flush changes + * after a while. This prevents the editor from getting stuck. + * + * Every setState call will have to go through this, otherwise we get race conditions + * where the underlying state has changed, but the draftState didn't change. + */ + setStateDebounced = ( + setStateFunc: + | Partial + | ((state: State, props: Props) => Partial), + time = 200 + ) => { + const draftState = this.draftState || this.state; + + const newState = + typeof setStateFunc === 'function' + ? setStateFunc(draftState, this.props) + : setStateFunc; + this.draftState = { ...draftState, ...newState }; + + if (this.setStateTimer) { + clearTimeout(this.setStateTimer); + } + + const updateFunc = () => { + if (this.draftState) { + this.setState(this.draftState); + } + + this.draftState = null; + this.setStateTimer = null; + }; + + if (time === 0) { + updateFunc(); + } else { + this.setStateTimer = window.setTimeout(updateFunc, time); + } + }; + normalizeHeight = (el: HTMLDivElement) => { if (typeof this.state.height === 'string') { const { height } = el.getBoundingClientRect(); @@ -204,7 +248,7 @@ export class DevTools extends React.PureComponent { setHidden = (hidden: boolean) => { if (!hidden) { - return this.setState(state => ({ + return this.setState((state) => ({ status: { ...state.status, [this.getCurrentPane().id]: null, @@ -228,39 +272,40 @@ export class DevTools extends React.PureComponent { status: 'success' | 'warning' | 'error' | 'info' | 'clear', count?: number ) => { - const currentStatus = (status !== 'clear' && this.state.status[id]) || { - unread: 0, - type: 'info', - }; - let newStatus = currentStatus.type; + this.setStateDebounced((state) => { + const currentStatus = (status !== 'clear' && state.status[id]) || { + unread: 0, + type: 'info', + }; + let newStatus = currentStatus.type; - if ( - status === 'success' && - newStatus !== 'error' && - newStatus !== 'warning' - ) { - newStatus = 'success'; - } else if (status === 'warning' && newStatus !== 'error') { - newStatus = 'warning'; - } else if (status === 'error') { - newStatus = 'error'; - } + if ( + status === 'success' && + newStatus !== 'error' && + newStatus !== 'warning' + ) { + newStatus = 'success'; + } else if (status === 'warning' && newStatus !== 'error') { + newStatus = 'warning'; + } else if (status === 'error') { + newStatus = 'error'; + } - let unread = currentStatus.unread + (status !== 'clear' ? 1 : 0); + let unread = currentStatus.unread + (status !== 'clear' ? 1 : 0); - if (count != null) { - unread = count; - } - - this.setState(state => ({ - status: { - ...state.status, - [id]: { - type: newStatus, - unread, + if (count != null) { + unread = count; + } + return { + status: { + ...state.status, + [id]: { + type: newStatus, + unread, + }, }, - }, - })); + }; + }, 50); }; handleTouchStart = (event: React.TouchEvent) => { @@ -276,7 +321,7 @@ export class DevTools extends React.PureComponent { const { clientY } = event; unFocus(document, window); // @ts-ignore - this.setState(state => ({ + this.setState((state) => ({ startY: clientY, startHeight: state.height, mouseDown: true, @@ -446,7 +491,7 @@ export class DevTools extends React.PureComponent { return ( { + ref={(el) => { this.node = el || this.node; if (this.node) { From f8389f414b5b94c29d932aad8dd3cfffe4e98a0b Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Tue, 8 Sep 2020 22:46:25 +0200 Subject: [PATCH 2/8] Fix clearing the console for the first time --- .../Preview/DevTools/Console/index.tsx | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/app/src/app/components/Preview/DevTools/Console/index.tsx b/packages/app/src/app/components/Preview/DevTools/Console/index.tsx index 3c39d7b2a7a..492bd1dfabd 100644 --- a/packages/app/src/app/components/Preview/DevTools/Console/index.tsx +++ b/packages/app/src/app/components/Preview/DevTools/Console/index.tsx @@ -27,10 +27,9 @@ const StyledClearIcon = styled(ClearIcon)` font-size: 0.8em; `; -class ConsoleComponent extends React.Component { +class ConsoleComponent extends React.PureComponent { state = { messages: [], - initialClear: true, filter: [], searchKeywords: '', }; @@ -53,7 +52,7 @@ class ConsoleComponent extends React.Component { } } - handleMessage = data => { + handleMessage = (data) => { switch (data.type) { case 'console': { const message = Decode(data.log); @@ -75,13 +74,8 @@ class ConsoleComponent extends React.Component { break; } case 'clear-console': { - if (this.state.initialClear) { - this.setState({ - initialClear: false, - }); - } else { - this.clearConsole(); - } + this.clearConsole(); + break; } case 'eval-result': { @@ -104,7 +98,7 @@ class ConsoleComponent extends React.Component { if (aggregatedResults) { const { summaryMessage, failedMessages } = aggregatedResults; this.addMessage('log', [summaryMessage]); - failedMessages.forEach(t => { + failedMessages.forEach((t) => { this.addMessage('warn', [t]); }); } else { @@ -146,7 +140,7 @@ class ConsoleComponent extends React.Component { this.props.updateStatus(this.getType(method)); } - this.setState(state => + this.setState((state) => update(state, { messages: { $push: [ @@ -232,7 +226,7 @@ class ConsoleComponent extends React.Component { return ( { + ref={(el) => { this.list = el; }} > @@ -260,7 +254,7 @@ const ConsoleFilterInput = ({ style }) => ( /> ); -const ConsoleFilterSelect = props => { +const ConsoleFilterSelect = (props) => { const handleOnChange = ({ target: { value } }) => { if (value === 'all') { dispatch({ type: 'filter-log', filters: [] }); From 3fcef06e436c6c0254d9112a48b7488ecc715775 Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Wed, 9 Sep 2020 21:00:48 +0200 Subject: [PATCH 3/8] Limit amount of messages we show in the console --- packages/app/package.json | 1 - .../Preview/DevTools/Console/index.tsx | 37 +++++++++++-------- .../app/src/app/overmind/internalActions.ts | 6 +-- yarn.lock | 2 +- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/app/package.json b/packages/app/package.json index ccc1b69f84c..0645ef9dd88 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -142,7 +142,6 @@ "humps": "CompuIves/humps", "ignore": "^5.1.4", "immer": "^3.2.0", - "immutability-helper": "^2.6.6", "instantsearch.css": "^7.1.0", "is-url": "^1.2.2", "jest-circus": "^22.1.4", diff --git a/packages/app/src/app/components/Preview/DevTools/Console/index.tsx b/packages/app/src/app/components/Preview/DevTools/Console/index.tsx index 492bd1dfabd..45292c77b29 100644 --- a/packages/app/src/app/components/Preview/DevTools/Console/index.tsx +++ b/packages/app/src/app/components/Preview/DevTools/Console/index.tsx @@ -2,7 +2,7 @@ import Select from '@codesandbox/common/lib/components/Select'; import theme from '@codesandbox/common/lib/theme'; import { listen, dispatch } from 'codesandbox-api'; import { Decode, Console as ConsoleFeed } from 'console-feed'; -import update from 'immutability-helper'; +import immer from 'immer'; import { debounce } from 'lodash-es'; import React from 'react'; import ClearIcon from 'react-icons/lib/md/block'; @@ -27,7 +27,18 @@ const StyledClearIcon = styled(ClearIcon)` font-size: 0.8em; `; -class ConsoleComponent extends React.PureComponent { +type State = { + messages: any[]; + filter: Array<'info' | 'log' | 'warn'>; + searchKeywords: string; +}; + +/** + * Max amount of messages that we show in the console + */ +const MAX_MESSAGE_COUNT = 500; + +class ConsoleComponent extends React.PureComponent { state = { messages: [], filter: [], @@ -36,7 +47,7 @@ class ConsoleComponent extends React.PureComponent { listener; - constructor(props) { + constructor(props: StyledProps) { super(props); this.scrollToBottom = debounce(this.scrollToBottom, 1 / 60); @@ -141,22 +152,18 @@ class ConsoleComponent extends React.PureComponent { } this.setState((state) => - update(state, { - messages: { - $push: [ - { - method, - data, - }, - ], - }, + immer(state, (draft) => { + draft.messages.push({ method, data }); + draft.messages = draft.messages.slice( + Math.max(0, draft.messages.length - MAX_MESSAGE_COUNT) + ); }) ); } - list; + list: HTMLDivElement | undefined; - UNSAFE_componentWillReceiveProps(nextProps) { + UNSAFE_componentWillReceiveProps(nextProps: StyledProps) { if (nextProps.sandboxId !== this.props.sandboxId) { this.clearConsole(true); } @@ -267,7 +274,7 @@ const ConsoleFilterSelect = (props) => { return (