Skip to content

Commit b3f5459

Browse files
authored
fix: Re-running ui code initially rendering the old document (#1017)
This fixes a bug where re-running code causes the old document to be rendered first. This can cause initial mounting with old props for document trees that don't change between runs which can cause problems with components that rely on an initial prop value only. This also fixes an issue where the old panels are shown until new panels/document is fetched. Now, it will show loading spinners on those panels instead. If the new document (re-assigned to the same variable in Python) has a new tree structure, then the old panels will be closed and new panels opened. If the document has the same tree, then the new panels will replace the old panels. This Python example shows a slow loading component. Run the code, then change the props (I commented out the `format_` prop) and re-run. The existing panel should turn into a loading spinner and then load without the formatting rules. ```py from deephaven import ui import deephaven.plot.express as dx import time @ui.component def my_t(): time.sleep(2) return ui.table( dx.data.stocks().update("SymColor=Sym==`FISH` ? `positive` : `salmon`"), hidden_columns=["SymColor"], format_=[ ui.TableFormat(value="0.00%"), ui.TableFormat(cols="Timestamp", value="E, dd MMM yyyy HH:mm:ss z"), ui.TableFormat(cols="Size", color="info", if_="Size < 10"), ui.TableFormat(cols="Size", color="notice", if_="Size > 100"), ui.TableFormat(cols=["Sym", "Exchange"], alignment="center"), ui.TableFormat( cols=["Sym", "Exchange"], background_color="negative", if_="Sym=`CAT`" ), ui.TableFormat(if_="Sym=`DOG`", color="oklab(0.6 -0.3 -0.25)"), ui.TableFormat(cols="Sym", color="SymColor"), ], ) t = my_t() ```
1 parent c25f578 commit b3f5459

File tree

3 files changed

+40
-16
lines changed

3 files changed

+40
-16
lines changed

plugins/ui/src/js/src/DashboardPlugin.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,7 @@ export function DashboardPlugin(
7676
>(new Map());
7777

7878
const handleWidgetOpen = useCallback(
79-
({
80-
widgetId = nanoid(),
81-
widget,
82-
}: {
83-
widgetId: string;
84-
widget: WidgetDescriptor;
85-
}) => {
79+
({ widgetId, widget }: { widgetId: string; widget: WidgetDescriptor }) => {
8680
log.debug('Opening widget with ID', widgetId, widget);
8781
setWidgetMap(prevWidgetMap => {
8882
const newWidgetMap = new Map(prevWidgetMap);

plugins/ui/src/js/src/layout/ReactPanel.tsx

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ import {
77
useLayoutManager,
88
useListener,
99
} from '@deephaven/dashboard';
10-
import { View, ViewProps, Flex, FlexProps } from '@deephaven/components';
10+
import {
11+
View,
12+
ViewProps,
13+
Flex,
14+
FlexProps,
15+
LoadingOverlay,
16+
} from '@deephaven/components';
1117
import Log from '@deephaven/log';
1218
import PortalPanel from './PortalPanel';
1319
import { ReactPanelControl, useReactPanel } from './ReactPanelManager';
@@ -186,6 +192,15 @@ function ReactPanel({
186192
);
187193
const widgetStatus = useWidgetStatus();
188194

195+
let renderedChildren: React.ReactNode;
196+
if (widgetStatus.status === 'loading') {
197+
renderedChildren = <LoadingOverlay />;
198+
} else if (widgetStatus.status === 'error') {
199+
renderedChildren = <WidgetErrorView error={widgetStatus.error} />;
200+
} else {
201+
renderedChildren = children;
202+
}
203+
189204
return portal
190205
? ReactDOM.createPortal(
191206
<ReactPanelContext.Provider value={panelId}>
@@ -224,11 +239,7 @@ function ReactPanel({
224239
* Don't render the children if there's an error with the widget. If there's an error with the widget, we can assume the children won't render properly,
225240
* but we still want the panels to appear so things don't disappear/jump around.
226241
*/}
227-
{widgetStatus.status === 'error' ? (
228-
<WidgetErrorView error={widgetStatus.error} />
229-
) : (
230-
children
231-
)}
242+
{renderedChildren}
232243
</ReactPanelErrorBoundary>
233244
</Flex>
234245
</View>

plugins/ui/src/js/src/widget/WidgetHandler.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import React, {
99
useRef,
1010
useState,
1111
} from 'react';
12+
// eslint-disable-next-line camelcase
13+
import { unstable_batchedUpdates } from 'react-dom';
1214
import {
1315
JSONRPCClient,
1416
JSONRPCServer,
@@ -64,6 +66,16 @@ function WidgetHandler({
6466
initialData: initialDataProp,
6567
}: WidgetHandlerProps): JSX.Element | null {
6668
const { widget, error: widgetError } = useWidget(widgetDescriptor);
69+
const [isLoading, setIsLoading] = useState(true);
70+
const [prevWidgetDescriptor, setPrevWidgetDescriptor] =
71+
useState(widgetDescriptor);
72+
// Cannot use usePrevious to change setIsLoading
73+
// Since usePrevious runs in an effect, the value never gets updated if setIsLoading is called during render
74+
// Use the widgetDescriptor because useWidget is async so the widget doesn't immediately change
75+
if (widgetDescriptor !== prevWidgetDescriptor) {
76+
setPrevWidgetDescriptor(widgetDescriptor);
77+
setIsLoading(true);
78+
}
6779

6880
const [document, setDocument] = useState<ReactNode>();
6981

@@ -224,8 +236,12 @@ function WidgetHandler({
224236
log.debug2(METHOD_DOCUMENT_UPDATED, params);
225237
const [documentParam, stateParam] = params;
226238
const newDocument = parseDocument(documentParam);
227-
setInternalError(undefined);
228-
setDocument(newDocument);
239+
// TODO: Remove unstable_batchedUpdates wrapper when upgrading to React 18
240+
unstable_batchedUpdates(() => {
241+
setInternalError(undefined);
242+
setDocument(newDocument);
243+
setIsLoading(false);
244+
});
229245
if (stateParam != null) {
230246
try {
231247
const newState = JSON.parse(stateParam);
@@ -339,11 +355,14 @@ function WidgetHandler({
339355
if (error != null) {
340356
return { status: 'error', descriptor: widgetDescriptor, error };
341357
}
358+
if (isLoading) {
359+
return { status: 'loading', descriptor: widgetDescriptor };
360+
}
342361
if (renderedDocument != null) {
343362
return { status: 'ready', descriptor: widgetDescriptor };
344363
}
345364
return { status: 'loading', descriptor: widgetDescriptor };
346-
}, [error, renderedDocument, widgetDescriptor]);
365+
}, [error, renderedDocument, widgetDescriptor, isLoading]);
347366

348367
return useMemo(
349368
() =>

0 commit comments

Comments
 (0)