Skip to content

Commit

Permalink
Merge pull request #2948 from quantified-uncertainty/viewer-provider-…
Browse files Browse the repository at this point in the history
…singleton

ViewerProvider is a singleton now
  • Loading branch information
OAGr authored Jan 9, 2024
2 parents 3ad42db + a4b2ef0 commit 591a245
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 98 deletions.
108 changes: 55 additions & 53 deletions packages/components/src/components/SquiggleOutputViewer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { ErrorBoundary } from "../ErrorBoundary.js";
import { PartialPlaygroundSettings } from "../PlaygroundSettings.js";
import { SquiggleErrorAlert } from "../SquiggleErrorAlert.js";
import { SquiggleViewerHandle } from "../SquiggleViewer/index.js";
import { ViewerProvider } from "../SquiggleViewer/ViewerProvider.js";
import { Layout } from "./Layout.js";
import { RenderingIndicator } from "./RenderingIndicator.js";

Expand Down Expand Up @@ -79,12 +80,8 @@ export const SquiggleOutputViewer = forwardRef<SquiggleViewerHandle, Props>(
<div className="absolute z-10 inset-0 bg-white opacity-50" />
)}
<ErrorBoundary>
<SquiggleViewer
{...settings}
ref={viewerRef}
value={usedResult.value}
editor={editor}
/>
{/* we don't pass settings or editor here because they're already configured in `<ViewerProvider>`; hopefully `<SquiggleViewer>` itself won't need to rely on settings, otherwise things might break */}
<SquiggleViewer ref={viewerRef} value={usedResult.value} />
</ErrorBoundary>
</div>
) : (
Expand All @@ -94,53 +91,58 @@ export const SquiggleOutputViewer = forwardRef<SquiggleViewerHandle, Props>(
}

return (
<Layout
menu={
<Dropdown
render={({ close }) => (
<DropdownMenu>
<DropdownMenuActionItem
icon={CodeBracketIcon}
title={
<MenuItemTitle
title="Variables"
type={variablesCount ? `{}${variablesCount}` : null}
/>
}
onClick={() => {
setMode("variables");
close();
}}
/>
<DropdownMenuActionItem
icon={CodeBracketIcon}
title={
<MenuItemTitle
title="Result"
type={hasResult ? "" : null}
/>
}
onClick={() => {
setMode("result");
close();
}}
/>
</DropdownMenu>
)}
>
<Button size="small">
<div className="flex items-center space-x-1.5">
<span>{mode === "variables" ? "Variables" : "Result"}</span>
<TriangleIcon className="rotate-180 text-slate-400" size={10} />
</div>
</Button>
</Dropdown>
}
indicator={
<RenderingIndicator isRunning={isRunning} output={squiggleOutput} />
}
viewer={squiggleViewer}
/>
<ViewerProvider partialPlaygroundSettings={settings} editor={editor}>
<Layout
menu={
<Dropdown
render={({ close }) => (
<DropdownMenu>
<DropdownMenuActionItem
icon={CodeBracketIcon}
title={
<MenuItemTitle
title="Variables"
type={variablesCount ? `{}${variablesCount}` : null}
/>
}
onClick={() => {
setMode("variables");
close();
}}
/>
<DropdownMenuActionItem
icon={CodeBracketIcon}
title={
<MenuItemTitle
title="Result"
type={hasResult ? "" : null}
/>
}
onClick={() => {
setMode("result");
close();
}}
/>
</DropdownMenu>
)}
>
<Button size="small">
<div className="flex items-center space-x-1.5">
<span>{mode === "variables" ? "Variables" : "Result"}</span>
<TriangleIcon
className="rotate-180 text-slate-400"
size={10}
/>
</div>
</Button>
</Dropdown>
}
indicator={
<RenderingIndicator isRunning={isRunning} output={squiggleOutput} />
}
viewer={squiggleViewer}
/>
</ViewerProvider>
);
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { SqValuePath } from "@quri/squiggle-lang";

import { useForceUpdate } from "../../lib/hooks/useForceUpdate.js";
import { useStabilizeObjectIdentity } from "../../lib/hooks/useStabilizeObject.js";
import { SqValueWithContext } from "../../lib/utility.js";
import { CalculatorState } from "../../widgets/CalculatorWidget/types.js";
import { CodeEditorHandle } from "../CodeEditor/index.js";
Expand Down Expand Up @@ -153,14 +154,15 @@ class ItemStore {
}

type ViewerContextShape = {
// Note that we don't store localItemState themselves in the context (that would cause rerenders of the entire tree on each settings update).
// Instead, we keep localItemState in local state and notify the global context via setLocalItemState to pass them down the component tree again if it got rebuilt from scratch.
// Note that we don't store `localItemState` itself in the context (that would cause rerenders of the entire tree on each settings update).
// Instead, we keep `localItemState` in local state and notify the global context via `setLocalItemState` to pass them down the component tree again if it got rebuilt from scratch.
// See ./SquiggleViewer.tsx and ./ValueWithContextViewer.tsx for other implementation details on this.
globalSettings: PlaygroundSettings;
focused: SqValuePath | undefined;
setFocused: (value: SqValuePath | undefined) => void;
editor?: CodeEditorHandle;
itemStore: ItemStore;
initialized: boolean;
};

export const ViewerContext = createContext<ViewerContextShape>({
Expand All @@ -169,6 +171,7 @@ export const ViewerContext = createContext<ViewerContextShape>({
setFocused: () => undefined,
editor: undefined,
itemStore: new ItemStore(),
initialized: false,
});

export function useViewerContext() {
Expand Down Expand Up @@ -293,39 +296,66 @@ export function useMergedSettings(path: SqValuePath) {
return result;
}

export const ViewerProvider = forwardRef<
SquiggleViewerHandle,
PropsWithChildren<{
partialPlaygroundSettings: PartialPlaygroundSettings;
editor?: CodeEditorHandle;
}>
>(({ partialPlaygroundSettings, editor, children }, ref) => {
const [itemStore] = useState(() => new ItemStore());

useImperativeHandle(ref, () => ({
viewValuePath(path: SqValuePath) {
itemStore.scrollToPath(path);
},
}));
type Props = PropsWithChildren<{
partialPlaygroundSettings: PartialPlaygroundSettings;
editor?: CodeEditorHandle;
}>;

const [focused, setFocused] = useState<SqValuePath | undefined>();
export const InnerViewerProvider = forwardRef<SquiggleViewerHandle, Props>(
(
{ partialPlaygroundSettings: unstablePlaygroundSettings, editor, children },
ref
) => {
const [itemStore] = useState(() => new ItemStore());

/**
* Because we often obtain `partialPlaygroundSettings` with spread syntax, its identity changes on each render, which could
* cause extra unnecessary re-renders of widgets, in some cases.
* Related discussion: https://github.com/quantified-uncertainty/squiggle/pull/2525#discussion_r1393398447
*/
const playgroundSettings = useStabilizeObjectIdentity(
unstablePlaygroundSettings
);

useImperativeHandle(ref, () => ({
viewValuePath(path: SqValuePath) {
itemStore.scrollToPath(path);
},
}));

const globalSettings = useMemo(() => {
return merge({}, defaultPlaygroundSettings, partialPlaygroundSettings);
}, [partialPlaygroundSettings]);
const [focused, setFocused] = useState<SqValuePath | undefined>();

const globalSettings = useMemo(() => {
return merge({}, defaultPlaygroundSettings, playgroundSettings);
}, [playgroundSettings]);

return (
<ViewerContext.Provider
value={{
globalSettings,
editor,
focused,
setFocused,
itemStore,
initialized: true,
}}
>
{children}
</ViewerContext.Provider>
);
}
);
InnerViewerProvider.displayName = "InnerViewerProvider";

export const ViewerProvider = forwardRef<SquiggleViewerHandle, Props>(
(props, ref) => {
// `ViewerProvider` is a singleton, so if the context already exists, we don't initialize it again
const { initialized } = useContext(ViewerContext);
if (initialized) {
return props.children; // TODO: `ref` and settings will be ignored
}

return (
<ViewerContext.Provider
value={{
globalSettings,
editor,
focused,
setFocused,
itemStore,
}}
>
{children}
</ViewerContext.Provider>
);
});
return <InnerViewerProvider ref={ref} {...props} />;
}
);
ViewerProvider.displayName = "ViewerProvider";
12 changes: 1 addition & 11 deletions packages/components/src/components/SquiggleViewer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { FC, forwardRef, memo } from "react";
import { SqValue, SqValuePath } from "@quri/squiggle-lang";
import { ChevronRightIcon } from "@quri/ui";

import { useStabilizeObjectIdentity } from "../../lib/hooks/useStabilizeObject.js";
import { MessageAlert } from "../Alert.js";
import { CodeEditorHandle } from "../CodeEditor/index.js";
import { PartialPlaygroundSettings } from "../PlaygroundSettings.js";
Expand Down Expand Up @@ -127,18 +126,9 @@ const component = forwardRef<SquiggleViewerHandle, SquiggleViewerProps>(
{ value, editor, ...partialPlaygroundSettings },
ref
) {
/**
* Because we obtain `partialPlaygroundSettings` with spread syntax, its identity changes on each render, which could
* cause extra unnecessary re-renders of widgets, in some cases.
* Related discussion: https://github.com/quantified-uncertainty/squiggle/pull/2525#discussion_r1393398447
*/
const stablePartialPlaygroundSettings = useStabilizeObjectIdentity(
partialPlaygroundSettings
);

return (
<ViewerProvider
partialPlaygroundSettings={stablePartialPlaygroundSettings}
partialPlaygroundSettings={partialPlaygroundSettings}
editor={editor}
ref={ref}
>
Expand Down

3 comments on commit 591a245

@vercel
Copy link

@vercel vercel bot commented on 591a245 Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 591a245 Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on 591a245 Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.