From 89927a37908fc9e84b981f1cba7c9abd37039c8b Mon Sep 17 00:00:00 2001 From: Mo Date: Sun, 30 Oct 2022 10:48:23 -0500 Subject: [PATCH] perf: avoid uneccessary notes list item rerenders (#1904) --- .../Components/Abstract/PureComponent.tsx | 2 +- .../ApplicationView/ApplicationView.tsx | 2 +- .../ContentListView/ContentList.tsx | 33 ++--- .../ContentListView/ContentListItem.tsx | 6 +- .../ContentListView/NoteListItem.tsx | 3 + .../Types/AbstractListItemProps.ts | 88 +++++++++++++- .../javascripts/Components/Footer/Footer.tsx | 4 +- .../NoteGroupView/NoteGroupView.tsx | 4 +- .../Components/NoteView/NoteView.tsx | 114 ++++++++++++------ .../PasswordWizard/PasswordWizard.tsx | 4 +- .../SyncResolutionMenu/SyncResolutionMenu.tsx | 4 +- .../{Navigation => Tags}/Navigation.tsx | 0 .../Components/Tags/TagsListItem.tsx | 3 + packages/web/src/javascripts/Logging.ts | 9 +- 14 files changed, 208 insertions(+), 68 deletions(-) rename packages/web/src/javascripts/Components/{Navigation => Tags}/Navigation.tsx (100%) diff --git a/packages/web/src/javascripts/Components/Abstract/PureComponent.tsx b/packages/web/src/javascripts/Components/Abstract/PureComponent.tsx index e9d8fb0bb..8d31e9fe7 100644 --- a/packages/web/src/javascripts/Components/Abstract/PureComponent.tsx +++ b/packages/web/src/javascripts/Components/Abstract/PureComponent.tsx @@ -7,7 +7,7 @@ import { Component } from 'react' export type PureComponentState = Partial> export type PureComponentProps = Partial> -export abstract class PureComponent

extends Component { +export abstract class AbstractComponent

extends Component { private unsubApp!: () => void private reactionDisposers: IReactionDisposer[] = [] diff --git a/packages/web/src/javascripts/Components/ApplicationView/ApplicationView.tsx b/packages/web/src/javascripts/Components/ApplicationView/ApplicationView.tsx index b4b4b7a0d..80381c722 100644 --- a/packages/web/src/javascripts/Components/ApplicationView/ApplicationView.tsx +++ b/packages/web/src/javascripts/Components/ApplicationView/ApplicationView.tsx @@ -4,7 +4,7 @@ import { ApplicationEvent, Challenge, removeFromArray, WebAppEvent } from '@stan import { PANEL_NAME_NOTES, PANEL_NAME_NAVIGATION } from '@/Constants/Constants' import { alertDialog, RouteType } from '@standardnotes/ui-services' import { WebApplication } from '@/Application/Application' -import Navigation from '@/Components/Navigation/Navigation' +import Navigation from '@/Components/Tags/Navigation' import NoteGroupView from '@/Components/NoteGroupView/NoteGroupView' import Footer from '@/Components/Footer/Footer' import SessionsModal from '@/Components/SessionsModal/SessionsModal' diff --git a/packages/web/src/javascripts/Components/ContentListView/ContentList.tsx b/packages/web/src/javascripts/Components/ContentListView/ContentList.tsx index 4220679b8..9bd7827e0 100644 --- a/packages/web/src/javascripts/Components/ContentListView/ContentList.tsx +++ b/packages/web/src/javascripts/Components/ContentListView/ContentList.tsx @@ -40,6 +40,7 @@ const ContentList: FunctionComponent = ({ const { selectPreviousItem, selectNextItem } = selectionController const { hideTags, hideDate, hideNotePreview, hideEditorIcon } = itemListController.webDisplayOptions const { sortBy } = itemListController.displayOptions + const selectedTag = navigationController.selected const onScroll: UIEventHandler = useCallback( (e) => { @@ -72,25 +73,27 @@ const ContentList: FunctionComponent = ({ [selectionController], ) - const getTagsForItem = (item: ListableContentItem) => { - if (hideTags) { - return [] - } + const getTagsForItem = useCallback( + (item: ListableContentItem) => { + if (hideTags) { + return [] + } - const selectedTag = navigationController.selected - if (!selectedTag) { - return [] - } + if (!selectedTag) { + return [] + } - const tags = application.getItemTags(item) + const tags = application.getItemTags(item) - const isNavigatingOnlyTag = selectedTag instanceof SNTag && tags.length === 1 - if (isNavigatingOnlyTag) { - return [] - } + const isNavigatingOnlyTag = selectedTag instanceof SNTag && tags.length === 1 + if (isNavigatingOnlyTag) { + return [] + } - return tags - } + return tags + }, + [hideTags, selectedTag, application], + ) return (

= (props) => { switch (props.item.content_type) { @@ -15,4 +15,4 @@ const ContentListItem: FunctionComponent = (props) => { } } -export default ContentListItem +export default React.memo(ContentListItem, (a, b) => !doListItemPropsMeritRerender(a, b)) diff --git a/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx b/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx index 46641d69e..9dc9fa832 100644 --- a/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx +++ b/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx @@ -13,6 +13,7 @@ import { AppPaneId } from '../ResponsivePane/AppPaneMetadata' import { useContextMenuEvent } from '@/Hooks/useContextMenuEvent' import ListItemNotePreviewText from './ListItemNotePreviewText' import { ListItemTitle } from './ListItemTitle' +import { log, LoggingDomain } from '@/Logging' const NoteListItem: FunctionComponent = ({ application, @@ -70,6 +71,8 @@ const NoteListItem: FunctionComponent = ({ useContextMenuEvent(listItemRef, openContextMenu) + log(LoggingDomain.ItemsList, 'Rendering note list item', item.title) + return (
t.title) + .sort() + .join() !== + next + .map((t) => t.title) + .sort() + .join() + ) { + return true + } + + return false +} + +function doesItemChangeMeritRerender(previous: ListableContentItem, next: ListableContentItem): boolean { + if (previous.uuid !== next.uuid) { + return true + } + + const propertiesMeritingRerender: (keyof ListableContentItem)[] = [ + 'title', + 'protected', + 'updatedAtString', + 'createdAtString', + 'hidePreview', + 'preview_html', + 'preview_plain', + 'archived', + 'starred', + 'pinned', + ] + + for (const key of propertiesMeritingRerender) { + if (previous[key] !== next[key]) { + return true + } + } + + return false +} diff --git a/packages/web/src/javascripts/Components/Footer/Footer.tsx b/packages/web/src/javascripts/Components/Footer/Footer.tsx index 88f79d0d2..32dc8615b 100644 --- a/packages/web/src/javascripts/Components/Footer/Footer.tsx +++ b/packages/web/src/javascripts/Components/Footer/Footer.tsx @@ -1,6 +1,6 @@ import { WebApplication } from '@/Application/Application' import { ApplicationGroup } from '@/Application/ApplicationGroup' -import { PureComponent } from '@/Components/Abstract/PureComponent' +import { AbstractComponent } from '@/Components/Abstract/PureComponent' import { destroyAllObjectProperties, preventRefreshing } from '@/Utils' import { ApplicationEvent, ApplicationDescriptor, WebAppEvent } from '@standardnotes/snjs' import { @@ -41,7 +41,7 @@ type State = { arbitraryStatusMessage?: string } -class Footer extends PureComponent { +class Footer extends AbstractComponent { public user?: unknown private didCheckForOffline = false private completedInitialSync = false diff --git a/packages/web/src/javascripts/Components/NoteGroupView/NoteGroupView.tsx b/packages/web/src/javascripts/Components/NoteGroupView/NoteGroupView.tsx index 3c63f7d35..872bca94e 100644 --- a/packages/web/src/javascripts/Components/NoteGroupView/NoteGroupView.tsx +++ b/packages/web/src/javascripts/Components/NoteGroupView/NoteGroupView.tsx @@ -1,5 +1,5 @@ import { FileItem, FileViewController, NoteViewController } from '@standardnotes/snjs' -import { PureComponent } from '@/Components/Abstract/PureComponent' +import { AbstractComponent } from '@/Components/Abstract/PureComponent' import { WebApplication } from '@/Application/Application' import MultipleSelectedNotes from '@/Components/MultipleSelectedNotes/MultipleSelectedNotes' import MultipleSelectedFiles from '../MultipleSelectedFiles/MultipleSelectedFiles' @@ -22,7 +22,7 @@ type Props = { application: WebApplication } -class NoteGroupView extends PureComponent { +class NoteGroupView extends AbstractComponent { private removeChangeObserver!: () => void constructor(props: Props) { diff --git a/packages/web/src/javascripts/Components/NoteView/NoteView.tsx b/packages/web/src/javascripts/Components/NoteView/NoteView.tsx index e59559182..283e25ce4 100644 --- a/packages/web/src/javascripts/Components/NoteView/NoteView.tsx +++ b/packages/web/src/javascripts/Components/NoteView/NoteView.tsx @@ -1,49 +1,50 @@ -import { ChangeEventHandler, createRef, KeyboardEventHandler, RefObject } from 'react' +import { AbstractComponent } from '@/Components/Abstract/PureComponent' +import ChangeEditorButton from '@/Components/ChangeEditor/ChangeEditorButton' +import ComponentView from '@/Components/ComponentView/ComponentView' +import NotesOptionsPanel from '@/Components/NotesOptions/NotesOptionsPanel' +import PanelResizer, { PanelResizeType, PanelSide } from '@/Components/PanelResizer/PanelResizer' +import PinNoteButton from '@/Components/PinNoteButton/PinNoteButton' +import ProtectedItemOverlay from '@/Components/ProtectedItemOverlay/ProtectedItemOverlay' +import { ElementIds } from '@/Constants/ElementIDs' +import { PrefDefaults } from '@/Constants/PrefDefaults' +import { StringDeleteNote, STRING_DELETE_LOCKED_ATTEMPT, STRING_DELETE_PLACEHOLDER_ATTEMPT } from '@/Constants/Strings' +import { log, LoggingDomain } from '@/Logging' +import { debounce, isDesktopApplication, isMobileScreen } from '@/Utils' +import { classNames } from '@/Utils/ConcatenateClassNames' import { ApplicationEvent, - isPayloadSourceRetrieved, - isPayloadSourceInternalChange, - ContentType, - SNComponent, - SNNote, ComponentArea, - PrefKey, ComponentViewerInterface, - ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction, + ContentType, + EditorFontSize, + EditorLineHeight, + isPayloadSourceInternalChange, + isPayloadSourceRetrieved, + NoteType, NoteViewController, PayloadEmitSource, + PrefKey, + ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction, + SNComponent, + SNNote, WebAppEvent, - EditorLineHeight, - EditorFontSize, - NoteType, } from '@standardnotes/snjs' -import { debounce, isDesktopApplication, isMobileScreen } from '@/Utils' +import { confirmDialog, KeyboardKey, KeyboardModifier } from '@standardnotes/ui-services' +import { ChangeEventHandler, createRef, KeyboardEventHandler, RefObject } from 'react' import { EditorEventSource } from '../../Types/EditorEventSource' -import { confirmDialog, KeyboardModifier, KeyboardKey } from '@standardnotes/ui-services' -import { STRING_DELETE_PLACEHOLDER_ATTEMPT, STRING_DELETE_LOCKED_ATTEMPT, StringDeleteNote } from '@/Constants/Strings' -import { PureComponent } from '@/Components/Abstract/PureComponent' -import ProtectedItemOverlay from '@/Components/ProtectedItemOverlay/ProtectedItemOverlay' -import PinNoteButton from '@/Components/PinNoteButton/PinNoteButton' -import NotesOptionsPanel from '@/Components/NotesOptions/NotesOptionsPanel' -import ComponentView from '@/Components/ComponentView/ComponentView' -import PanelResizer, { PanelSide, PanelResizeType } from '@/Components/PanelResizer/PanelResizer' -import { ElementIds } from '@/Constants/ElementIDs' -import ChangeEditorButton from '@/Components/ChangeEditor/ChangeEditorButton' +import IndicatorCircle from '../IndicatorCircle/IndicatorCircle' +import LinkedItemBubblesContainer from '../LinkedItems/LinkedItemBubblesContainer' +import LinkedItemsButton from '../LinkedItems/LinkedItemsButton' +import MobileItemsListButton from '../NoteGroupView/MobileItemsListButton' import EditingDisabledBanner from './EditingDisabledBanner' +import { reloadFont } from './FontFunctions' +import NoteStatusIndicator, { NoteStatus } from './NoteStatusIndicator' +import NoteViewFileDropTarget from './NoteViewFileDropTarget' +import { NoteViewProps } from './NoteViewProps' import { transactionForAssociateComponentWithCurrentNote, transactionForDisassociateComponentWithCurrentNote, } from './TransactionFunctions' -import { reloadFont } from './FontFunctions' -import { NoteViewProps } from './NoteViewProps' -import IndicatorCircle from '../IndicatorCircle/IndicatorCircle' -import { classNames } from '@/Utils/ConcatenateClassNames' -import MobileItemsListButton from '../NoteGroupView/MobileItemsListButton' -import LinkedItemBubblesContainer from '../LinkedItems/LinkedItemBubblesContainer' -import NoteStatusIndicator, { NoteStatus } from './NoteStatusIndicator' -import { PrefDefaults } from '@/Constants/PrefDefaults' -import LinkedItemsButton from '../LinkedItems/LinkedItemsButton' -import NoteViewFileDropTarget from './NoteViewFileDropTarget' const MinimumStatusDuration = 400 const TextareaDebounce = 100 @@ -98,7 +99,7 @@ const PlaintextFontSizeMapping: Record = { Large: 'text-xl', } -class NoteView extends PureComponent { +class NoteView extends AbstractComponent { readonly controller!: NoteViewController private statusTimeout?: NodeJS.Timeout @@ -193,7 +194,7 @@ class NoteView extends PureComponent { ;(this.onPanelResizeFinish as unknown) = undefined ;(this.stackComponentExpanded as unknown) = undefined ;(this.toggleStackComponent as unknown) = undefined - ;(this.onSystemEditorLoad as unknown) = undefined + ;(this.onSystemEditorRef as unknown) = undefined ;(this.debounceReloadEditorComponent as unknown) = undefined ;(this.textAreaChangeDebounceSave as unknown) = undefined ;(this.editorContentRef as unknown) = undefined @@ -207,6 +208,24 @@ class NoteView extends PureComponent { return this.controller.item } + override shouldComponentUpdate(_nextProps: Readonly, nextState: Readonly): boolean { + const complexObjects: (keyof State)[] = ['availableStackComponents', 'stackComponentViewers'] + for (const key of Object.keys(nextState) as (keyof State)[]) { + if (complexObjects.includes(key)) { + continue + } + const prevValue = this.state[key] + const nextValue = nextState[key] + + if (prevValue !== nextValue) { + log(LoggingDomain.NoteView, 'Rendering due to state change', key, prevValue, nextValue) + return true + } + } + + return false + } + override componentDidMount(): void { super.componentDidMount() @@ -253,6 +272,8 @@ class NoteView extends PureComponent { } onNoteInnerChange(note: SNNote, source: PayloadEmitSource): void { + log(LoggingDomain.NoteView, 'On inner note change', PayloadEmitSource[source]) + if (note.uuid !== this.note.uuid) { throw Error('Editor received changes for non-current note') } @@ -431,12 +452,15 @@ class NoteView extends PureComponent { streamItems() { this.removeComponentStreamObserver = this.application.streamItems(ContentType.Component, async ({ source }) => { + log(LoggingDomain.NoteView, 'On component stream observer', PayloadEmitSource[source]) if (isPayloadSourceInternalChange(source) || source === PayloadEmitSource.InitialObserverRegistrationPush) { return } + if (!this.note) { return } + await this.reloadStackComponents() this.debounceReloadEditorComponent() }) @@ -454,6 +478,7 @@ class NoteView extends PureComponent { if (this.state.editorComponentViewerDidAlreadyReload && !force) { return } + const component = viewer.component this.application.componentManager.destroyComponentViewer(viewer) this.setState( @@ -489,6 +514,7 @@ class NoteView extends PureComponent { } async reloadEditorComponent() { + log(LoggingDomain.NoteView, 'Reload editor component') if (this.state.showProtectedWarning) { this.destroyCurrentEditorComponent() return @@ -581,13 +607,16 @@ class NoteView extends PureComponent { onTextAreaChange: ChangeEventHandler = ({ currentTarget }) => { const text = currentTarget.value + this.setState({ editorText: text, }) + this.textAreaChangeDebounceSave() } textAreaChangeDebounceSave = () => { + log(LoggingDomain.NoteView, 'Performing save after debounce') this.controller .save({ editorValues: { @@ -609,10 +638,14 @@ class NoteView extends PureComponent { } onTitleChange: ChangeEventHandler = ({ currentTarget }) => { + log(LoggingDomain.NoteView, 'Performing save after title change') + const title = currentTarget.value + this.setState({ editorTitle: title, }) + this.controller .save({ editorValues: { @@ -662,10 +695,12 @@ class NoteView extends PureComponent { this.application.alertService.alert(STRING_DELETE_PLACEHOLDER_ATTEMPT).catch(console.error) return } + if (this.note.locked) { this.application.alertService.alert(STRING_DELETE_LOCKED_ATTEMPT).catch(console.error) return } + const title = this.note.title.length ? `'${this.note.title}'` : 'this note' const text = StringDeleteNote(title, permanently) if ( @@ -727,6 +762,7 @@ class NoteView extends PureComponent { } async reloadPreferences() { + log(LoggingDomain.NoteView, 'Reload preferences') const monospaceFont = this.application.getPreference( PrefKey.EditorMonospaceEnabled, PrefDefaults[PrefKey.EditorMonospaceEnabled], @@ -776,9 +812,8 @@ class NoteView extends PureComponent { } } - /** @components */ - async reloadStackComponents() { + log(LoggingDomain.NoteView, 'Reload stack components') const stackComponents = sortAlphabetically( this.application.componentManager .componentsForArea(ComponentArea.EditorStack) @@ -844,10 +879,13 @@ class NoteView extends PureComponent { }) } - onSystemEditorLoad = (ref: HTMLTextAreaElement | null) => { + onSystemEditorRef = (ref: HTMLTextAreaElement | null) => { if (this.removeTabObserver || !ref) { return } + + log(LoggingDomain.NoteView, 'On system editor ref') + /** * Insert 4 spaces when a tab key is pressed, * only used when inside of the text editor. @@ -1070,7 +1108,7 @@ class NoteView extends PureComponent { onFocus={this.onContentFocus} onBlur={this.onContentBlur} readOnly={this.state.noteLocked} - ref={(ref) => ref && this.onSystemEditorLoad(ref)} + ref={(ref) => ref && this.onSystemEditorRef(ref)} spellCheck={this.state.spellcheck} value={this.state.editorText} className={classNames( diff --git a/packages/web/src/javascripts/Components/PasswordWizard/PasswordWizard.tsx b/packages/web/src/javascripts/Components/PasswordWizard/PasswordWizard.tsx index afc195b49..e92ef7528 100644 --- a/packages/web/src/javascripts/Components/PasswordWizard/PasswordWizard.tsx +++ b/packages/web/src/javascripts/Components/PasswordWizard/PasswordWizard.tsx @@ -1,6 +1,6 @@ import { WebApplication } from '@/Application/Application' import { createRef } from 'react' -import { PureComponent } from '@/Components/Abstract/PureComponent' +import { AbstractComponent } from '@/Components/Abstract/PureComponent' import Button from '@/Components/Button/Button' import DecoratedPasswordInput from '../Input/DecoratedPasswordInput' import ModalDialog from '../Shared/ModalDialog' @@ -38,7 +38,7 @@ type FormData = { status?: string } -class PasswordWizard extends PureComponent { +class PasswordWizard extends AbstractComponent { private currentPasswordInput = createRef() constructor(props: Props) { diff --git a/packages/web/src/javascripts/Components/SyncResolutionMenu/SyncResolutionMenu.tsx b/packages/web/src/javascripts/Components/SyncResolutionMenu/SyncResolutionMenu.tsx index 6a5cbafe2..6dffee17e 100644 --- a/packages/web/src/javascripts/Components/SyncResolutionMenu/SyncResolutionMenu.tsx +++ b/packages/web/src/javascripts/Components/SyncResolutionMenu/SyncResolutionMenu.tsx @@ -1,12 +1,12 @@ import { WebApplication } from '@/Application/Application' -import { PureComponent } from '@/Components/Abstract/PureComponent' +import { AbstractComponent } from '@/Components/Abstract/PureComponent' type Props = { application: WebApplication close: () => void } -class SyncResolutionMenu extends PureComponent { +class SyncResolutionMenu extends AbstractComponent { constructor(props: Props) { super(props, props.application) } diff --git a/packages/web/src/javascripts/Components/Navigation/Navigation.tsx b/packages/web/src/javascripts/Components/Tags/Navigation.tsx similarity index 100% rename from packages/web/src/javascripts/Components/Navigation/Navigation.tsx rename to packages/web/src/javascripts/Components/Tags/Navigation.tsx diff --git a/packages/web/src/javascripts/Components/Tags/TagsListItem.tsx b/packages/web/src/javascripts/Components/Tags/TagsListItem.tsx index 31ea4aaf4..bbc5a603c 100644 --- a/packages/web/src/javascripts/Components/Tags/TagsListItem.tsx +++ b/packages/web/src/javascripts/Components/Tags/TagsListItem.tsx @@ -28,6 +28,7 @@ import { mergeRefs } from '@/Hooks/mergeRefs' import { useFileDragNDrop } from '../FileDragNDropProvider/FileDragNDropProvider' import { LinkingController } from '@/Controllers/LinkingController' import { TagListSectionType } from './TagListSection' +import { log, LoggingDomain } from '@/Logging' type Props = { tag: SNTag @@ -237,6 +238,8 @@ export const TagsListItem: FunctionComponent = observer( } }, [addDragTarget, linkingController, removeDragTarget, tag]) + log(LoggingDomain.NavigationList, 'Rendering TagsListItem') + return ( <>
= { [LoggingDomain.DailyNotes]: false, + [LoggingDomain.NoteView]: false, + [LoggingDomain.ItemsList]: false, + [LoggingDomain.NavigationList]: false, } export function log(domain: LoggingDomain, ...args: any[]): void { - if (!LoggingStatus[domain]) { + if (!isDev || !LoggingStatus[domain]) { return }