Skip to content

Commit e888f51

Browse files
stipsanrexxars
authored andcommittedApr 14, 2023
fix: refactor desk tool intent resolver to reduce remounts (#4363)
* fix: refactor desk tool intent resolver to reduce remounts * refactor(router): speed up `useRouterState`, reduce total renders * fix: reduce renders in components with `useRouter` * fix: reduce spinner "popping" when loading the desk tool * refactor: use the selector in useRouterState instead of separate useMemo * refactor(desktool): replace toast with loading pane
1 parent 72b37a5 commit e888f51

File tree

15 files changed

+121
-158
lines changed

15 files changed

+121
-158
lines changed
 

‎packages/sanity/src/core/studio/StudioLayout.tsx

+15-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Card, Flex, Spinner} from '@sanity/ui'
1+
import {Card, Flex, Spinner, Text} from '@sanity/ui'
22
import {startCase} from 'lodash'
33
import React, {
44
createContext,
@@ -15,7 +15,7 @@ import {ToolNotFoundScreen} from './screens/ToolNotFoundScreen'
1515
import {useNavbarComponent} from './studio-components-hooks'
1616
import {StudioErrorBoundary} from './StudioErrorBoundary'
1717
import {useWorkspace} from './workspace'
18-
import {RouteScope, useRouter} from 'sanity/router'
18+
import {RouteScope, useRouterState} from 'sanity/router'
1919

2020
const SearchFullscreenPortalCard = styled(Card)`
2121
height: 100%;
@@ -43,10 +43,17 @@ export const NavbarContext = createContext<NavbarContextValue>({
4343

4444
/** @public */
4545
export function StudioLayout() {
46-
const {state: routerState} = useRouter()
4746
const {name, title, tools} = useWorkspace()
48-
const activeToolName = typeof routerState.tool === 'string' ? routerState.tool : undefined
49-
const activeTool = tools.find((tool) => tool.name === activeToolName)
47+
const activeToolName = useRouterState(
48+
useCallback(
49+
(routerState) => (typeof routerState.tool === 'string' ? routerState.tool : undefined),
50+
[]
51+
)
52+
)
53+
const activeTool = useMemo(
54+
() => tools.find((tool) => tool.name === activeToolName),
55+
[activeToolName, tools]
56+
)
5057
const [searchFullscreenOpen, setSearchFullscreenOpen] = useState<boolean>(false)
5158
const [searchFullscreenPortalEl, setSearchFullscreenPortalEl] = useState<HTMLDivElement | null>(
5259
null
@@ -110,9 +117,11 @@ export function StudioLayout() {
110117
)
111118
}
112119

120+
// @TODO re-use `LoadingComponent`, which is `LoadingScreen` by default, to reduce "popping" during initial load
113121
function LoadingTool() {
114122
return (
115-
<Flex align="center" height="fill" justify="center">
123+
<Flex justify="center" align="center" height="fill" direction="column" gap={4}>
124+
<Text muted>Loading…</Text>
116125
<Spinner muted />
117126
</Flex>
118127
)

‎packages/sanity/src/core/studio/components/navbar/new-document/NewDocumentListOption.tsx

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {CurrentUser} from '@sanity/types'
22
import {Tooltip, Box, Card, Text} from '@sanity/ui'
3-
import React, {useCallback} from 'react'
3+
import React, {useCallback, useMemo} from 'react'
44
import styled from 'styled-components'
55
import {InsufficientPermissionsMessage} from '../../../../components'
66
import {NewDocumentOption, PreviewLayout} from './types'
@@ -25,9 +25,13 @@ interface NewDocumentListOptionProps {
2525

2626
export function NewDocumentListOption(props: NewDocumentListOptionProps) {
2727
const {option, currentUser, onClick, preview} = props
28+
const params = useMemo(
29+
() => ({template: option.templateId, type: option.schemaType}),
30+
[option.schemaType, option.templateId]
31+
)
2832
const {onClick: onIntentClick, href} = useIntentLink({
2933
intent: 'create',
30-
params: {template: option.templateId, type: option.schemaType},
34+
params,
3135
})
3236

3337
const handleDocumentClick = useCallback(

‎packages/sanity/src/core/studio/components/navbar/search/components/searchResults/item/SearchResultItem.tsx

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {Box, ResponsiveMarginProps, ResponsivePaddingProps} from '@sanity/ui'
2-
import React, {MouseEvent, useCallback} from 'react'
2+
import React, {MouseEvent, useCallback, useMemo} from 'react'
33
import {PreviewCard} from '../../../../../../../components'
44
import {useSchema} from '../../../../../../../hooks'
55
import {useDocumentPresence} from '../../../../../../../store'
@@ -26,12 +26,10 @@ export function SearchResultItem({
2626
const type = schema.get(documentType)
2727
const documentPresence = useDocumentPresence(documentId)
2828

29+
const params = useMemo(() => ({id: documentId, type: type?.name}), [documentId, type?.name])
2930
const {onClick: onIntentClick, href} = useIntentLink({
3031
intent: 'edit',
31-
params: {
32-
id: documentId,
33-
type: type?.name,
34-
},
32+
params,
3533
})
3634

3735
const handleClick = useCallback(

‎packages/sanity/src/core/studio/components/navbar/tools/ToolLink.tsx

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, {forwardRef, useMemo} from 'react'
1+
import React, {forwardRef, useCallback} from 'react'
22
import {StateLink, useRouterState} from 'sanity/router'
33

44
/** @beta */
@@ -13,11 +13,8 @@ export const ToolLink = forwardRef(function ToolLink(
1313
ref: React.ForwardedRef<HTMLAnchorElement>
1414
) {
1515
const {name, ...rest} = props
16-
const routerState = useRouterState()
17-
18-
const state = useMemo(
19-
() => ({...routerState, tool: name, [name]: undefined}),
20-
[routerState, name]
16+
const state = useRouterState(
17+
useCallback((routerState) => ({...routerState, tool: name, [name]: undefined}), [name])
2118
)
2219

2320
return <StateLink state={state} {...rest} ref={ref} />

‎packages/sanity/src/core/studio/router/helpers.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ export function resolveDefaultState(
4646

4747
export function resolveIntentState(
4848
tools: Tool[],
49-
currentState: RouterState | null,
50-
intentState: RouterState
49+
prevState: RouterState | null,
50+
nextState: RouterState
5151
): RouterEvent {
52-
const {intent, params, payload} = intentState
52+
const {intent, params, payload} = nextState
5353

5454
if (typeof intent !== 'string') {
5555
throw new Error('intent must be a string')
@@ -61,31 +61,31 @@ export function resolveIntentState(
6161

6262
const orderedTools = getOrderedTools(tools)
6363

64-
const currentTool = currentState?.tool
65-
? orderedTools.find((tool) => tool.name === currentState.tool)
64+
const currentTool = prevState?.tool
65+
? orderedTools.find((tool) => tool.name === prevState.tool)
6666
: null
6767

6868
// If current tool can handle intent and if so, give it precedence
6969
const matchingTool = (currentTool ? [currentTool, ...orderedTools] : orderedTools).find(
7070
(tool) =>
7171
tool &&
7272
typeof tool.canHandleIntent === 'function' &&
73-
tool.canHandleIntent(intent, params, currentState && currentState[tool.name])
73+
tool.canHandleIntent(intent, params, prevState && prevState[tool.name])
7474
)
7575

7676
if (matchingTool?.getIntentState) {
7777
const toolState = matchingTool.getIntentState(
7878
intent,
7979
params as any,
80-
currentState && (currentState[matchingTool.name] as any),
80+
prevState && (prevState[matchingTool.name] as any),
8181
payload
8282
)
8383

8484
return {
8585
type: 'state',
8686
isNotFound: false,
8787
state: {
88-
...currentState,
88+
...prevState,
8989
tool: matchingTool.name,
9090
[matchingTool.name]: toolState,
9191
},

‎packages/sanity/src/desk/components/deskTool/DeskTool.tsx

+13
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {PaneLayout} from '../pane'
1010
import {useDeskTool} from '../../useDeskTool'
1111
import {NoDocumentTypesScreen} from './NoDocumentTypesScreen'
1212
import {useSchema, _isCustomDocumentTypeDefinition} from 'sanity'
13+
import {useRouterState} from 'sanity/router'
1314

1415
interface DeskToolProps {
1516
onPaneChange: (panes: Array<PaneNode | typeof LOADING_PANE>) => void
@@ -30,6 +31,12 @@ export const DeskTool = memo(function DeskTool({onPaneChange}: DeskToolProps) {
3031
const schema = useSchema()
3132
const {layoutCollapsed, setLayoutCollapsed} = useDeskTool()
3233
const {paneDataItems, resolvedPanes} = useResolvedPanes()
34+
// Intent resolving is processed by the sibling `<IntentResolver />` but it doesn't have a UI for indicating progress.
35+
// We handle that here, so if there are only 1 pane (the root structure), and there's an intent state in the router, we need to show a placeholder LoadingPane until
36+
// the structure is resolved and we know what panes to load/display
37+
const isResolvingIntent = useRouterState(
38+
useCallback((routerState) => typeof routerState.intent === 'string', [])
39+
)
3340

3441
const [portalElement, setPortalElement] = useState<HTMLDivElement | null>(null)
3542

@@ -109,13 +116,19 @@ export const DeskTool = memo(function DeskTool({onPaneChange}: DeskToolProps) {
109116
paneKey={paneKey}
110117
params={paneParams}
111118
payload={payload}
119+
path={path}
112120
selected={selected}
113121
siblingIndex={siblingIndex}
114122
/>
115123
)}
116124
</Fragment>
117125
)
118126
)}
127+
{/* If there's just 1 pane (the root), or less, and we're resolving an intent then it's necessary to show */}
128+
{/* a loading indicator as the intent resolving is async, could take a while and can also be interrupted/redirected */}
129+
{paneDataItems.length <= 1 && isResolvingIntent && (
130+
<LoadingPane paneKey="intent-resolver" />
131+
)}
119132
</StyledPaneLayout>
120133
<div data-portal="" ref={setPortalElement} />
121134
</PortalProvider>

‎packages/sanity/src/desk/components/deskTool/DeskToolBoundary.tsx

+4-17
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
import React, {useEffect, useMemo, useState} from 'react'
1+
import React, {useEffect, useState} from 'react'
22
import {ErrorBoundary} from '@sanity/ui'
33
import {DeskToolOptions} from '../../types'
44
import {DeskToolProvider} from '../../DeskToolProvider'
55
import {setActivePanes} from '../../getIntentState'
66
import {IntentResolver} from './intentResolver'
77
import {StructureError} from './StructureError'
88
import {DeskTool} from './DeskTool'
9-
import {useRouter} from 'sanity/router'
10-
import {SourceProvider, useWorkspace, Tool, isRecord} from 'sanity'
11-
12-
const EMPTY_RECORD = {}
9+
import {SourceProvider, useWorkspace, Tool} from 'sanity'
1310

1411
interface DeskToolBoundaryProps {
1512
tool: Tool<DeskToolOptions>
@@ -20,15 +17,6 @@ export function DeskToolBoundary({tool: {options}}: DeskToolBoundaryProps) {
2017
const [firstSource] = sources
2118
const {source, defaultDocumentNode, structure} = options || {}
2219

23-
const {state: routerState} = useRouter()
24-
const intent = useMemo(() => {
25-
const intentName = typeof routerState.intent === 'string' ? routerState.intent : undefined
26-
const params = isRecord(routerState.params) ? routerState.params : EMPTY_RECORD
27-
const payload = routerState.payload
28-
29-
return intentName ? {intent: intentName, params, payload} : undefined
30-
}, [routerState])
31-
3220
// Set active panes to blank on mount and unmount
3321
useEffect(() => {
3422
setActivePanes([])
@@ -43,9 +31,8 @@ export function DeskToolBoundary({tool: {options}}: DeskToolBoundaryProps) {
4331
<ErrorBoundary onCatch={setError}>
4432
<SourceProvider name={source || firstSource.name}>
4533
<DeskToolProvider defaultDocumentNode={defaultDocumentNode} structure={structure}>
46-
{/* when an intent is found, we render the intent resolver component */}
47-
{/* which asynchronously resolves the intent then navigates to it */}
48-
{intent ? <IntentResolver {...intent} /> : <DeskTool onPaneChange={setActivePanes} />}
34+
<DeskTool onPaneChange={setActivePanes} />
35+
<IntentResolver />
4936
</DeskToolProvider>
5037
</SourceProvider>
5138
</ErrorBoundary>
Original file line numberDiff line numberDiff line change
@@ -1,82 +1,73 @@
1-
import {Box, Card, Flex, Spinner, Text} from '@sanity/ui'
2-
import React, {useEffect, useState} from 'react'
1+
import {memo, useCallback, useEffect, useState} from 'react'
32
import {resolveIntent} from '../../../structureResolvers'
4-
import {RouterPanes} from '../../../types'
53
import {useDeskTool} from '../../../useDeskTool'
6-
import {Delay} from '../../Delay'
7-
import {Redirect} from './Redirect'
84
import {ensureDocumentIdAndType} from './utils'
9-
import {useDocumentStore, useUnique} from 'sanity'
5+
import {useRouter, useRouterState} from 'sanity/router'
6+
import {isRecord, useDocumentStore} from 'sanity'
107

11-
export interface IntentResolverProps {
12-
intent: string
13-
params: Record<string, unknown> // {type: string; id: string; [key: string]: string | undefined}
14-
payload: unknown
15-
}
8+
const EMPTY_RECORD: Record<string, unknown> = {}
169

1710
/**
1811
* A component that receives an intent from props and redirects to the resolved
1912
* intent location (while showing a loading spinner during the process)
2013
*/
21-
export function IntentResolver({
22-
intent,
23-
params: paramsProp = {},
24-
payload: payloadProp,
25-
}: IntentResolverProps) {
14+
export const IntentResolver = memo(function IntentResolver() {
15+
const {navigate} = useRouter()
16+
const maybeIntent = useRouterState(
17+
useCallback((routerState) => {
18+
const intentName = typeof routerState.intent === 'string' ? routerState.intent : undefined
19+
return intentName
20+
? {
21+
intent: intentName,
22+
params: isRecord(routerState.params) ? routerState.params : EMPTY_RECORD,
23+
payload: routerState.payload,
24+
}
25+
: undefined
26+
}, [])
27+
)
2628
const {rootPaneNode, structureContext} = useDeskTool()
2729
const documentStore = useDocumentStore()
28-
const params = useUnique(paramsProp)
29-
const payload = useUnique(payloadProp)
30-
const [nextRouterPanes, setNextRouterPanes] = useState<RouterPanes | null>(null)
3130
const [error, setError] = useState<unknown>(null)
32-
const idParam = typeof params.id === 'string' ? params.id : undefined
33-
const typeParam = typeof params.type === 'string' ? params.type : undefined
3431

32+
// this re-throws errors so that parent ErrorBoundary's can handle them properly
33+
if (error) throw error
34+
35+
// eslint-disable-next-line consistent-return
3536
useEffect(() => {
36-
const cancelledRef = {current: false}
37+
if (maybeIntent) {
38+
const {intent, params, payload} = maybeIntent
3739

38-
async function getNextRouterPanes() {
39-
const {id, type} = await ensureDocumentIdAndType(documentStore, idParam, typeParam)
40+
let cancelled = false
41+
// eslint-disable-next-line no-inner-declarations
42+
async function effect() {
43+
const {id, type} = await ensureDocumentIdAndType(
44+
documentStore,
45+
typeof params.id === 'string' ? params.id : undefined,
46+
typeof params.type === 'string' ? params.type : undefined
47+
)
4048

41-
return resolveIntent({
42-
intent,
43-
params: {...params, id, type},
44-
payload,
45-
rootPaneNode,
46-
structureContext,
47-
})
48-
}
49+
if (cancelled) return
4950

50-
getNextRouterPanes()
51-
.then((result) => {
52-
if (!cancelledRef.current) {
53-
setNextRouterPanes(result)
54-
}
55-
})
56-
.catch(setError)
51+
const panes = await resolveIntent({
52+
intent,
53+
params: {...params, id, type},
54+
payload,
55+
rootPaneNode,
56+
structureContext,
57+
})
5758

58-
return () => {
59-
cancelledRef.current = true
60-
}
61-
}, [documentStore, idParam, intent, params, payload, rootPaneNode, structureContext, typeParam])
59+
if (cancelled) return
6260

63-
// throwing here bubbles the error up to the error boundary inside of the
64-
// `DeskToolRoot` component
65-
if (error) throw error
66-
if (nextRouterPanes) return <Redirect panes={nextRouterPanes} />
61+
navigate({panes}, {replace: true})
62+
}
6763

68-
return (
69-
<Card height="fill">
70-
<Delay ms={300}>
71-
<Flex align="center" direction="column" height="fill" justify="center">
72-
<Spinner muted />
73-
<Box marginTop={3}>
74-
<Text align="center" muted size={1}>
75-
Loading…
76-
</Text>
77-
</Box>
78-
</Flex>
79-
</Delay>
80-
</Card>
81-
)
82-
}
64+
effect().catch(setError)
65+
66+
return () => {
67+
cancelled = true
68+
}
69+
}
70+
}, [documentStore, maybeIntent, navigate, rootPaneNode, structureContext])
71+
72+
return null
73+
})

0 commit comments

Comments
 (0)
Failed to load comments.