Cleanup compile results on compilation error (#5663)

- pdfUrl, pdfDownloadUrl and log entries are now cleared up after a compilation error.

- Added a new hook, useScopeValueSetterOnly that limits how changes in React components are propagated to Angular. Since the compilation process happens entirely within compile-context it make sense that compilation results are solely managed by compile-context itself.

GitOrigin-RevId: 2003be03bab6753bb9a88194c7a6163cfd36f9c2

services/web/frontend/js/features/pdf-preview/util/compiler.js

@@ -22,6 +22,7 @@ export default class DocumentCompiler {
     setData,
     setFirstRenderDone,
     setError,
+    cleanupCompileResult,
     signal,
   }) {
     this.project = project
@@ -30,6 +31,7 @@ export default class DocumentCompiler {
     this.setData = setData
     this.setFirstRenderDone = setFirstRenderDone
     this.setError = setError
+    this.cleanupCompileResult = cleanupCompileResult
     this.signal = signal
 
     this.clsiServerId = null
@@ -100,9 +102,13 @@ export default class DocumentCompiler {
       const { firstRenderDone } = trackPdfDownload(data, compileTimeClientE2E)
       this.setFirstRenderDone(() => firstRenderDone)
       data.options = options
+      if (data.clsiServerId) {
+        this.clsiServerId = data.clsiServerId
+      }
       this.setData(data)
     } catch (error) {
       console.error(error)
+      this.cleanupCompileResult()
       this.setError(error.info?.statusCode === 429 ? 'rate-limited' : 'error')
     } finally {
       this.setCompiling(false)

services/web/frontend/js/shared/context/compile-context.js

@@ -8,6 +8,7 @@ import {
 } from 'react'
 import PropTypes from 'prop-types'
 import useScopeValue from '../hooks/use-scope-value'
+import useScopeValueSetterOnly from '../hooks/use-scope-value-setter-only'
 import usePersistedState from '../hooks/use-persisted-state'
 import useAbortController from '../hooks/use-abort-controller'
 import DocumentCompiler from '../../features/pdf-preview/util/compiler'
@@ -37,10 +38,8 @@ CompileContext.Provider.propTypes = {
     error: PropTypes.string,
     fileList: PropTypes.object,
     hasChanges: PropTypes.bool.isRequired,
-    hasLintingError: PropTypes.bool,
     highlights: PropTypes.arrayOf(PropTypes.object),
     logEntries: PropTypes.object,
-    logEntryAnnotations: PropTypes.object,
     pdfDownloadUrl: PropTypes.string,
     pdfUrl: PropTypes.string,
     pdfViewer: PropTypes.string,
@@ -75,21 +74,21 @@ export function CompileProvider({ children }) {
   const [compiling, setCompiling] = useState(false)
 
   // the log entries parsed from the compile output log
-  const [logEntries, setLogEntries] = useScopeValue('pdf.logEntries')
+  const [logEntries, setLogEntries] = useScopeValueSetterOnly('pdf.logEntries')
 
   // annotations for display in the editor, built from the log entries
-  const [logEntryAnnotations, setLogEntryAnnotations] = useScopeValue(
-    'pdf.logEntryAnnotations'
-  )
+  const [, setLogEntryAnnotations] = useScopeValue('pdf.logEntryAnnotations')
 
   // the PDF viewer
   const [pdfViewer] = useScopeValue('settings.pdfViewer')
 
   // the URL for downloading the PDF
-  const [pdfDownloadUrl, setPdfDownloadUrl] = useScopeValue('pdf.downloadUrl')
+  const [pdfDownloadUrl, setPdfDownloadUrl] = useScopeValueSetterOnly(
+    'pdf.downloadUrl'
+  )
 
   // the URL for loading the PDF in the preview pane
-  const [pdfUrl, setPdfUrl] = useScopeValue('pdf.url')
+  const [pdfUrl, setPdfUrl] = useScopeValueSetterOnly('pdf.url')
 
   // the project is considered to be "uncompiled" if a doc has changed since the last compile started
   const [uncompiled, setUncompiled] = useScopeValue('pdf.uncompiled')
@@ -161,6 +160,13 @@ export function CompileProvider({ children }) {
 
   const { signal } = useAbortController()
 
+  const cleanupCompileResult = useCallback(() => {
+    setPdfUrl(null)
+    setPdfDownloadUrl(null)
+    setLogEntries(null)
+    setLogEntryAnnotations({})
+  }, [setPdfUrl, setPdfDownloadUrl, setLogEntries, setLogEntryAnnotations])
+
   // the document compiler
   const [compiler] = useState(() => {
     return new DocumentCompiler({
@@ -170,6 +176,7 @@ export function CompileProvider({ children }) {
       setData,
       setFirstRenderDone,
       setError,
+      cleanupCompileResult,
       signal,
     })
   })
@@ -215,11 +222,6 @@ export function CompileProvider({ children }) {
     if (data) {
       if (data.clsiServerId) {
         setClsiServerId(data.clsiServerId) // set in scope, for PdfSynctexController
-        compiler.clsiServerId = data.clsiServerId
-      }
-
-      if (data.compileGroup) {
-        compiler.compileGroup = data.compileGroup
       }
 
       if (data.outputFiles) {
@@ -304,7 +306,6 @@ export function CompileProvider({ children }) {
       }
     }
   }, [
-    compiler,
     data,
     ide,
     hasPremiumCompile,
@@ -422,10 +423,8 @@ export function CompileProvider({ children }) {
       error,
       fileList,
       hasChanges,
-      hasLintingError,
       highlights,
       logEntries,
-      logEntryAnnotations,
       pdfDownloadUrl,
       pdfUrl,
       pdfViewer,
@@ -433,7 +432,6 @@ export function CompileProvider({ children }) {
       rawLog,
       recompileFromScratch,
       setAutoCompile,
-      setClearingCache,
       setCompiling,
       setDraft,
       setError,
@@ -461,10 +459,8 @@ export function CompileProvider({ children }) {
       error,
       fileList,
       hasChanges,
-      hasLintingError,
       highlights,
       logEntries,
-      logEntryAnnotations,
       position,
       pdfDownloadUrl,
       pdfUrl,
@@ -474,7 +470,7 @@ export function CompileProvider({ children }) {
       setAutoCompile,
       setDraft,
       setError,
-      setHasLintingError,
+      setHasLintingError, // only for stories
       setHighlights,
       setPosition,
       setStopOnValidationError,

services/web/frontend/js/shared/hooks/use-scope-value-setter-only.js

@@ -0,0 +1,37 @@
+import PropTypes from 'prop-types'
+import { useCallback, useState } from 'react'
+import { useIdeContext } from '../context/ide-context'
+import _ from 'lodash'
+
+/**
+ * Similar to `useScopeValue`, but instead of creating a two-way binding, only
+ * changes in react-> angular direction are propagated, with `value` remaining
+ * local and independent of its value in the Angular scope.
+ *
+ * The interface is compatible with React.useState(), including
+ * the option of passing a function to the setter.
+ *
+ * @param {string} path - dot '.' path of a property in the Angular scope.
+ * @param {any} defaultValue
+ * @returns {[any, function]} - value and setter function tuple.
+ */
+export default function useScopeValueSetterOnly(path, defaultValue) {
+  const { $scope } = useIdeContext({
+    $scope: PropTypes.object.isRequired,
+  })
+
+  const [value, setValue] = useState(defaultValue)
+
+  const scopeSetter = useCallback(
+    newValue => {
+      setValue(val => {
+        const actualNewValue = _.isFunction(newValue) ? newValue(val) : newValue
+        $scope.$applyAsync(() => _.set($scope, path, actualNewValue))
+        return actualNewValue
+      })
+    },
+    [path, $scope]
+  )
+
+  return [value, scopeSetter]
+}

services/web/test/frontend/features/pdf-preview/components/pdf-preview.test.js

@@ -482,20 +482,12 @@ describe('<PdfPreview/>', function () {
         code: 'AWFUL_ERROR',
       })
 
-    const { rerender } = renderWithEditorContext(<PdfPreview />, { scope })
+    renderWithEditorContext(<PdfPreview />, { scope })
 
     await screen.findByText('Something went wrong while rendering this PDF.')
     expect(screen.queryByLabelText('Page 1')).to.not.exist
 
     expect(nock.isDone()).to.be.true
-
-    mockValidPdf()
-
-    rerender(<PdfPreview />)
-
-    await screen.findByLabelText('Page 1')
-    expect(screen.queryByText('Something went wrong while rendering this PDF.'))
-      .to.not.exist
   })
 
   it('shows an error for a corrupt PDF', async function () {
@@ -506,19 +498,11 @@ describe('<PdfPreview/>', function () {
       .get(/^\/build\/output.pdf/)
       .replyWithFile(200, corruptPDF)
 
-    const { rerender } = renderWithEditorContext(<PdfPreview />, { scope })
+    renderWithEditorContext(<PdfPreview />, { scope })
 
     await screen.findByText('Something went wrong while rendering this PDF.')
     expect(screen.queryByLabelText('Page 1')).to.not.exist
 
     expect(nock.isDone()).to.be.true
-
-    mockValidPdf()
-
-    rerender(<PdfPreview />)
-
-    await screen.findByLabelText('Page 1')
-    expect(screen.queryByText('Something went wrong while rendering this PDF.'))
-      .to.not.exist
   })
 })