From 87a16a41d86b2ef1ebb25fa5a36b0edcfd6de6c7 Mon Sep 17 00:00:00 2001 From: RheeseyB <1044774+Rheeseyb@users.noreply.github.com> Date: Tue, 13 Oct 2020 15:41:46 +0100 Subject: [PATCH] Kicking a problem a little further down the line (#610) * fix(editor) Extract built-in modules to a separate file for better resolving * fix(editor) Tie built in versions to those in our package.json * fix(editor) Moved error and console log tracking into OpenFileEditor * chore(tests) Isolating the failing to test * chore(tests) Now run both performance tests * wtf * fix(Editor) Remove setTimeout around console logging * chore(Editor) Pulled out 2 hooks from OpenFileEditor --- .../components/editor/editor-component.tsx | 154 +++++++++--------- .../performance-regression-tests.spec.tsx | 4 +- 2 files changed, 82 insertions(+), 76 deletions(-) diff --git a/editor/src/components/editor/editor-component.tsx b/editor/src/components/editor/editor-component.tsx index 75511599c14e..d658c0c3afde 100644 --- a/editor/src/components/editor/editor-component.tsx +++ b/editor/src/components/editor/editor-component.tsx @@ -74,8 +74,6 @@ const EmptyArray: Array = [] const ConsoleLogSizeLimit = 100 const EmptyConsoleLogs: Array = [] -let consoleLogTimeoutID: number | null = null -let canvasConsoleLogsVariable: Array = EmptyConsoleLogs export const EditorComponentInner = betterReactMemo( 'EditorComponentInner', @@ -194,63 +192,6 @@ export const EditorComponentInner = betterReactMemo( [updateDeltaWidth], ) - const [runtimeErrors, setRuntimeErrors] = React.useState>(EmptyArray) - - const onRuntimeError = React.useCallback( - (editedFile: string, error: FancyError, errorInfo?: React.ErrorInfo) => { - setRuntimeErrors([ - { - editedFile: editedFile, - error: error, - errorInfo: Utils.defaultIfNull(null, errorInfo), - }, - ]) - }, - [], - ) - - const clearRuntimeErrors = React.useCallback(() => { - setRuntimeErrors(EmptyArray) - }, []) - - const [canvasConsoleLogsState, setCanvasConsoleLogsState] = React.useState>( - [], - ) - - const modifyLogs = React.useCallback( - (updateLogs: (logs: Array) => Array) => { - const updatedLogs = updateLogs(canvasConsoleLogsVariable) - if (updatedLogs !== canvasConsoleLogsVariable) { - canvasConsoleLogsVariable = updateLogs(canvasConsoleLogsVariable) - if (consoleLogTimeoutID != null) { - clearTimeout(consoleLogTimeoutID) - } - consoleLogTimeoutID = setTimeout(() => { - setCanvasConsoleLogsState(canvasConsoleLogsVariable) - consoleLogTimeoutID = null - }) - } - }, - [setCanvasConsoleLogsState], - ) - - const clearConsoleLogs = React.useCallback(() => { - modifyLogs((_) => EmptyConsoleLogs) - }, [modifyLogs]) - - const addToConsoleLogs = React.useCallback( - (log: ConsoleLog) => { - modifyLogs((logs) => { - let result = [...logs, log] - while (result.length > ConsoleLogSizeLimit) { - result.shift() - } - return result - }) - }, - [modifyLogs], - ) - const canvasIsLive = useEditorState((store) => isLiveMode(store.editor.mode)) const toggleLiveCanvas = React.useCallback( @@ -392,14 +333,7 @@ export const EditorComponentInner = betterReactMemo( overflowX: 'hidden', }} > - + {/* insert more columns here */} @@ -491,16 +425,76 @@ const HelpTriangle = () => ( ) -interface OpenFileEditorProps { +function useRuntimeErrors(): { runtimeErrors: Array onRuntimeError: (editedFile: string, error: FancyError, errorInfo?: React.ErrorInfo) => void clearRuntimeErrors: () => void - canvasConsoleLogs: Array - clearConsoleLogs: () => void +} { + const [runtimeErrors, setRuntimeErrors] = React.useState>(EmptyArray) + + const onRuntimeError = React.useCallback( + (editedFile: string, error: FancyError, errorInfo?: React.ErrorInfo) => { + setRuntimeErrors([ + { + editedFile: editedFile, + error: error, + errorInfo: Utils.defaultIfNull(null, errorInfo), + }, + ]) + }, + [], + ) + + const clearRuntimeErrors = React.useCallback(() => { + setRuntimeErrors(EmptyArray) + }, []) + + return { + runtimeErrors: runtimeErrors, + onRuntimeError: onRuntimeError, + clearRuntimeErrors: clearRuntimeErrors, + } +} + +function useConsoleLogs(): { + consoleLogs: Array addToConsoleLogs: (log: ConsoleLog) => void + clearConsoleLogs: () => void +} { + const [consoleLogs, setConsoleLogs] = React.useState>(EmptyConsoleLogs) + + const modifyLogs = React.useCallback( + (updateLogs: (logs: Array) => Array) => { + setConsoleLogs(updateLogs) + }, + [setConsoleLogs], + ) + + const clearConsoleLogs = React.useCallback(() => { + modifyLogs((_) => EmptyConsoleLogs) + }, [modifyLogs]) + + const addToConsoleLogs = React.useCallback( + (log: ConsoleLog) => { + modifyLogs((logs) => { + let result = [...logs, log] + while (result.length > ConsoleLogSizeLimit) { + result.shift() + } + return result + }) + }, + [modifyLogs], + ) + + return { + consoleLogs: consoleLogs, + addToConsoleLogs: addToConsoleLogs, + clearConsoleLogs: clearConsoleLogs, + } } -const OpenFileEditor = betterReactMemo('OpenFileEditor', (props: OpenFileEditorProps) => { +const OpenFileEditor = betterReactMemo('OpenFileEditor', () => { const { noFileOpen, isUiJsFileOpen, @@ -517,20 +511,32 @@ const OpenFileEditor = betterReactMemo('OpenFileEditor', (props: OpenFileEditorP } }) + const { runtimeErrors, onRuntimeError, clearRuntimeErrors } = useRuntimeErrors() + const { consoleLogs, addToConsoleLogs, clearConsoleLogs } = useConsoleLogs() + if (noFileOpen) { return No file open } else if (areReleaseNotesOpen) { return } else if (isUiJsFileOpen) { - return + return ( + + ) } else if (isUserConfigurationOpen) { return } else { return ( ) } diff --git a/editor/src/core/performance/performance-regression-tests.spec.tsx b/editor/src/core/performance/performance-regression-tests.spec.tsx index 3e20640b5022..a23ffd37d6c6 100644 --- a/editor/src/core/performance/performance-regression-tests.spec.tsx +++ b/editor/src/core/performance/performance-regression-tests.spec.tsx @@ -59,8 +59,8 @@ describe('React Render Count Tests - ', () => { ) const renderCountAfter = renderResult.getNumberOfRenders() - expect(renderCountAfter - renderCountBefore).toBeGreaterThan(750) // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toBeLessThan(760) + expect(renderCountAfter - renderCountBefore).toBeGreaterThan(695) // if this breaks, GREAT NEWS but update the test please :) + expect(renderCountAfter - renderCountBefore).toBeLessThan(705) }) it('Changing the selected view', async () => {