Do not mutate plot props when zooming

webview/src/plots/components/App.test.tsx

@@ -3040,10 +3040,7 @@ describe('App', () => {
         const smoothPlot = screen.getByTestId(`plot_${smoothId}`)
         await waitForVega(smoothPlot)
 
-        // eslint-disable-next-line testing-library/no-node-access
-        const slider = smoothPlot.querySelector(
-          '.vega-bindings input[name="smooth"]'
-        )
+        const slider = within(smoothPlot).getByRole('slider')
         expect(slider).toBeInTheDocument()
 
         fireEvent.change(slider as HTMLInputElement, { target: { value: 0.4 } })
@@ -3063,10 +3060,7 @@ describe('App', () => {
         const popup = screen.getByTestId('zoomed-in-plot')
         await waitForVega(popup)
 
-        // eslint-disable-next-line testing-library/no-node-access
-        const slider = popup.querySelector(
-          '.vega-bindings input[name="smooth"]'
-        )
+        const slider = within(popup).getByRole('slider')
         expect(slider).toBeInTheDocument()
 
         fireEvent.change(slider as HTMLInputElement, { target: { value: 0.4 } })
@@ -3082,10 +3076,7 @@ describe('App', () => {
         const smoothPlot = screen.getByTestId(`plot_${smoothId}`)
         await waitForVega(smoothPlot)
 
-        // eslint-disable-next-line testing-library/no-node-access
-        const slider = smoothPlot.querySelector(
-          '.vega-bindings input[name="smooth"]'
-        )
+        const slider = within(smoothPlot).getByRole('slider')
         expect(slider).toBeInTheDocument()
 
         expect(slider).toHaveValue('0.6')
@@ -3105,10 +3096,8 @@ describe('App', () => {
         const popup = screen.getByTestId('zoomed-in-plot')
         await waitForVega(popup)
 
-        // eslint-disable-next-line testing-library/no-node-access
-        const slider = popup.querySelector(
-          '.vega-bindings input[name="smooth"]'
-        )
+        const slider = within(popup).getByRole('slider')
+
         expect(slider).toBeInTheDocument()
 
         expect(slider).toHaveValue('0.6')
@@ -3123,10 +3112,7 @@ describe('App', () => {
 
         await waitForVega(smoothPlot)
 
-        // eslint-disable-next-line testing-library/no-node-access
-        const slider = smoothPlot.querySelector(
-          '.vega-bindings input[name="smooth"]'
-        )
+        const slider = within(smoothPlot).getByRole('slider')
 
         expect(slider).toBeInTheDocument()
         expect(slider).toHaveValue('0.2')
@@ -3138,9 +3124,7 @@ describe('App', () => {
           }
         })
 
-        await waitFor(() => expect(slider).toHaveValue('0.7'), {
-          timeout: 5000
-        })
+        expect(slider).toHaveValue('0.7')
       })
     })
 

webview/src/plots/components/Plots.tsx

@@ -50,18 +50,13 @@ const PlotsContent = () => {
     return <EmptyState>Loading Plots...</EmptyState>
   }
 
-  const modal = zoomedInPlot?.plot && (
+  const modal = zoomedInPlot && (
     <Modal
       onClose={() => {
         dispatch(setZoomedInPlot(undefined))
       }}
     >
-      <ZoomedInPlot
-        isTemplatePlot={zoomedInPlot.isTemplatePlot}
-        id={zoomedInPlot.id}
-        props={zoomedInPlot.plot}
-        openActionsMenu={zoomedInPlot.openActionsMenu}
-      />
+      <ZoomedInPlot {...zoomedInPlot} />
     </Modal>
   )
 

webview/src/plots/components/ZoomablePlot.tsx

@@ -1,13 +1,13 @@
 import { PlotsSection } from 'dvc/src/plots/webview/contract'
 import { SpecWithTitles } from 'dvc/src/plots/vega/util'
-import React, { useEffect, useRef } from 'react'
+import React from 'react'
 import { useDispatch } from 'react-redux'
-import VegaLite, { VegaLiteProps } from 'react-vega/lib/VegaLite'
+import VegaLite from 'react-vega/lib/VegaLite'
 import { ZoomablePlotWrapper } from './ZoomablePlotWrapper'
 import { TemplateVegaLite } from './templatePlots/TemplateVegaLite'
 import { setZoomedInPlot } from './webviewSlice'
 import styles from './styles.module.scss'
-import { config } from './constants'
+import { createPlotProps } from './constants'
 import { changeDragAndDropMode } from './util'
 import { zoomPlot } from '../util/messages'
 import { useGetPlot } from '../hooks/useGetPlot'
@@ -29,35 +29,13 @@ export const ZoomablePlot: React.FC<ZoomablePlotProps> = ({
 }) => {
   const { data, spec, isTemplatePlot } = useGetPlot(section, id)
   const dispatch = useDispatch()
-  const currentPlotProps = useRef<VegaLiteProps>()
 
-  const plotProps: VegaLiteProps = {
-    actions: false,
-    config,
-    data,
-    'data-testid': `${id}-vega`,
-    renderer: 'svg',
-    spec
-  } as VegaLiteProps
-  currentPlotProps.current = plotProps
-
-  useEffect(() => {
-    dispatch(
-      setZoomedInPlot({
-        id,
-        isTemplatePlot,
-        plot: currentPlotProps.current,
-        refresh: true
-      })
-    )
-  }, [data, spec, dispatch, id, isTemplatePlot])
+  const plotProps = createPlotProps(data, id, spec)
 
   const handleOnClick = (openActionsMenu?: boolean) => {
     zoomPlot()
 
-    return dispatch(
-      setZoomedInPlot({ id, isTemplatePlot, openActionsMenu, plot: plotProps })
-    )
+    return dispatch(setZoomedInPlot({ id, isTemplatePlot, openActionsMenu }))
   }
 
   if (!data && !spec) {
@@ -103,16 +81,15 @@ export const ZoomablePlot: React.FC<ZoomablePlotProps> = ({
         >
           <Ellipsis />
         </span>
-        {currentPlotProps.current &&
-          (isTemplatePlot ? (
-            <TemplateVegaLite
-              vegaLiteProps={plotProps}
-              id={id}
-              onNewView={onNewView}
-            />
-          ) : (
-            <VegaLite {...plotProps} onNewView={onNewView} />
-          ))}
+        {isTemplatePlot ? (
+          <TemplateVegaLite
+            vegaLiteProps={plotProps}
+            id={id}
+            onNewView={onNewView}
+          />
+        ) : (
+          <VegaLite {...plotProps} onNewView={onNewView} />
+        )}
       </button>
     </ZoomablePlotWrapper>
   )

webview/src/plots/components/ZoomedInPlot.tsx

@@ -1,5 +1,6 @@
+import { PlotsSection } from 'dvc/src/plots/webview/contract'
 import React, { useEffect, useRef } from 'react'
-import VegaLite, { VegaLiteProps } from 'react-vega/lib/VegaLite'
+import VegaLite from 'react-vega/lib/VegaLite'
 import { Config } from 'vega-lite'
 import merge from 'lodash.merge'
 import cloneDeep from 'lodash.clonedeep'
@@ -9,6 +10,7 @@ import {
   reverseOfLegendSuppressionUpdate
 } from 'dvc/src/plots/vega/util'
 import { View } from 'react-vega'
+import { createPlotProps } from './constants'
 import { TemplateVegaLite } from './templatePlots/TemplateVegaLite'
 import styles from './styles.module.scss'
 import { ZoomablePlotWrapper } from './ZoomablePlotWrapper'
@@ -24,10 +26,10 @@ import {
   exportPlotDataAsJson,
   exportPlotDataAsTsv
 } from '../util/messages'
+import { useGetPlot } from '../hooks/useGetPlot'
 
 type ZoomedInPlotProps = {
   id: string
-  props: VegaLiteProps
   isTemplatePlot: boolean
   openActionsMenu?: boolean
 }
@@ -49,16 +51,13 @@ const appendActionToVega = (
 
 export const ZoomedInPlot: React.FC<ZoomedInPlotProps> = ({
   id,
-  props,
   isTemplatePlot,
   openActionsMenu
 }: ZoomedInPlotProps) => {
-  const isCustomPlot = !isTemplatePlot
-  const hasSmoothing =
-    isTemplatePlot &&
-    (props.spec as { params?: { name: string }[] }).params?.[0]?.name ===
-      'smooth'
-
+  const { data, spec } = useGetPlot(
+    isTemplatePlot ? PlotsSection.TEMPLATE_PLOTS : PlotsSection.CUSTOM_PLOTS,
+    id
+  )
   const zoomedInPlotRef = useRef<HTMLDivElement>(null)
 
   useEffect(() => {
@@ -70,6 +69,16 @@ export const ZoomedInPlot: React.FC<ZoomedInPlotProps> = ({
     }
   }, [])
 
+  if (!spec) {
+    return
+  }
+
+  const props = createPlotProps(data, id, spec)
+  const isCustomPlot = !isTemplatePlot
+  const hasSmoothing =
+    isTemplatePlot &&
+    (spec as { params?: { name: string }[] }).params?.[0]?.name === 'smooth'
+
   const onNewView = (view: View) => {
     const actions: HTMLDivElement | null | undefined =
       zoomedInPlotRef.current?.querySelector('.vega-actions')

webview/src/plots/components/constants.ts

@@ -1,4 +1,6 @@
 import { Config } from 'vega-lite'
+import { PlainObject, VisualizationSpec } from 'react-vega'
+import { VegaLiteProps } from 'react-vega/lib/VegaLite'
 import { ThemeProperty } from '../../util/styles'
 
 export const PlOT_FOREGROUND_COLOR = `var(${ThemeProperty.FOREGROUND_COLOR})`
@@ -13,7 +15,7 @@ const title = {
   fontWeight: PLOT_FONT_WEIGHT
 }
 
-export const config: Config = {
+const config: Config = {
   axis: {
     domain: false,
     gridColor: PlOT_FOREGROUND_COLOR,
@@ -42,3 +44,17 @@ export const config: Config = {
     subtitleColor: PlOT_FOREGROUND_COLOR
   }
 }
+
+export const createPlotProps = (
+  data: PlainObject | undefined,
+  id: string,
+  spec: VisualizationSpec | undefined
+) =>
+  ({
+    actions: false,
+    config,
+    data,
+    'data-testid': `${id}-vega`,
+    renderer: 'svg',
+    spec
+  }) as VegaLiteProps

webview/src/plots/components/templatePlots/TemplateVegaLite.tsx

@@ -5,11 +5,6 @@ import { View } from 'react-vega'
 import { PlotsState } from '../../store'
 import { setSmoothPlotValues } from '../../util/messages'
 
-interface VegaState {
-  signals?: { [name: string]: number | undefined }
-  data?: unknown
-}
-
 export const TemplateVegaLite = ({
   id,
   onNewView,
@@ -20,68 +15,47 @@ export const TemplateVegaLite = ({
   vegaLiteProps: VegaLiteProps
 }) => {
   const vegaView = useRef<View>()
-  const plotWrapperEl = useRef<HTMLSpanElement>(null)
   const smoothPlotValues = useSelector(
     (state: PlotsState) => state.template.smoothPlotValues
   )
-  const changeDebounceTimer = useRef(0)
+  const currentValue = smoothPlotValues[id]
 
   useEffect(() => {
-    const newValue = smoothPlotValues[id]
-    if (!newValue || !vegaView.current) {
-      return
+    return () => {
+      vegaView.current?.finalize()
     }
+  }, [])
 
-    const currentState: VegaState = vegaView.current.getState()
-    const currentValue: number | undefined = currentState?.signals?.smooth
-    if (newValue !== currentValue) {
-      vegaView.current.setState({
-        ...currentState,
-        signals: { ...currentState.signals, smooth: newValue }
-      })
+  useEffect(() => {
+    if (!currentValue || !vegaView.current) {
+      return
     }
-  }, [smoothPlotValues, id])
-
-  const addRangeEventListener = () => {
-    const smoothRange = plotWrapperEl.current?.querySelector(
-      'input[name="smooth"]'
-    )
-
-    smoothRange?.addEventListener('change', (event: Event) => {
-      if (event.target) {
-        window.clearTimeout(changeDebounceTimer.current)
-        changeDebounceTimer.current = window.setTimeout(() => {
-          setSmoothPlotValues(
-            id,
-            Number((event.target as HTMLInputElement).value)
-          )
-        }, 500)
-      }
-    })
-  }
+    if (vegaView.current.signal('smooth') !== currentValue) {
+      vegaView.current.signal('smooth', currentValue)
+      vegaView.current.run()
+    }
+  }, [currentValue])
 
   return (
-    <span ref={plotWrapperEl}>
+    <span>
       <VegaLite
         {...vegaLiteProps}
         onNewView={view => {
           onNewView(view)
           vegaView.current = view
-          const defaultValue = smoothPlotValues[id]
-          const state = view.getState() as VegaState
 
-          if (!state?.signals?.smooth) {
+          if (!view.signal('smooth')) {
             return
           }
 
-          if (defaultValue) {
-            view.setState({
-              ...state,
-              signals: { ...state.signals, smooth: defaultValue }
-            })
+          if (currentValue) {
+            view.signal('smooth', currentValue)
+            view.run()
           }
 
-          addRangeEventListener()
+          view.addSignalListener('smooth', (_, value) => {
+            setSmoothPlotValues(id, Number(value))
+          })
         }}
       />
     </span>