Merge pull request #256 from briefercloud/run-inputs-before-blocks

Run inputs before blocks
briefercloud · Nov 20, 2024 · a830a43 · a830a43
2 parents 2f78b9c + 2376824
commit a830a43
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 17 deletions.
diff --git a/apps/api/src/yjs/v2/observers/blocks/date-input.ts b/apps/api/src/yjs/v2/observers/blocks/date-input.ts
@@ -46,6 +46,8 @@ export class DateInputObserver implements IDateInputObserver {
     if (status !== 'idle') {
       block.setAttribute('status', 'idle')
     }
+
+    this.executor.save(block)
   }
 
   public async handleBlockEvent(

diff --git a/apps/api/src/yjs/v2/observers/blocks/input.ts b/apps/api/src/yjs/v2/observers/blocks/input.ts
@@ -65,6 +65,8 @@ export class InputObserver implements IInputObserver {
         status: 'idle',
       })
     }
+
+    this.executor.saveValue(block)
   }
 
   public async handleBlockEvent(

diff --git a/apps/api/src/yjs/v2/persistors.ts b/apps/api/src/yjs/v2/persistors.ts
@@ -27,6 +27,7 @@ import {
   isDateInputBlock,
   getDateInputAttributes,
   PivotTableBlock,
+  isTextInputBlock,
 } from '@briefer/editor'
 import { equals, omit } from 'ramda'
 import { uuidv4 } from 'lib0/random.js'
@@ -526,7 +527,8 @@ export class AppPersistor implements Persistor {
           if (!publishedBlock) {
             throw new Error(`Block(${blockId}) not found in published state`)
           }
-          if (!isInputBlock(publishedBlock)) {
+
+          if (!isTextInputBlock(publishedBlock)) {
             return true
           }
 

diff --git a/packages/editor/src/blocks/index.test.ts b/packages/editor/src/blocks/index.test.ts
@@ -86,7 +86,8 @@ describe('computeDepencyQueue', () => {
       clickedBlock,
       layout,
       blocks,
-      envStartedAt
+      envStartedAt,
+      false
     ).map((b) => b.getAttribute('id'))
 
     const expected = [secondBlock].map((b) => b.getAttribute('id'))

diff --git a/packages/editor/src/blocks/index.ts b/packages/editor/src/blocks/index.ts
@@ -36,7 +36,7 @@ import {
   getInputBlockExecutedAt,
   inputRequestSaveValue,
 } from './input.js'
-import { YBlockGroup, switchBlockType } from '../index.js'
+import { YBlockGroup, isTextInputBlock, switchBlockType } from '../index.js'
 import { FileUploadBlock, duplicateFileUploadBlock } from './fileUpload.js'
 import {
   DropdownInputBlock,
@@ -122,9 +122,13 @@ export const requestRun = <B extends YBlock>(
   skipDependencyCheck: boolean,
   customOnRequestRun?: (block: B) => void
 ) => {
-  const dependencies = skipDependencyCheck
-    ? []
-    : computeDepencyQueue(block, layout, blocks, environmentStartedAt)
+  const dependencies = computeDepencyQueue(
+    block,
+    layout,
+    blocks,
+    environmentStartedAt,
+    skipDependencyCheck
+  )
 
   const queue = dependencies
   if (!customOnRequestRun) {
@@ -158,9 +162,13 @@ export const requestTrySuggestion = (
   environmentStartedAt: Date | null,
   skipDependencyCheck = false
 ) => {
-  const dependencies = skipDependencyCheck
-    ? []
-    : computeDepencyQueue(block, layout, blocks, environmentStartedAt)
+  const dependencies = computeDepencyQueue(
+    block,
+    layout,
+    blocks,
+    environmentStartedAt,
+    skipDependencyCheck
+  )
   const queue = dependencies.concat(block)
 
   for (const block of queue) {
@@ -334,6 +342,22 @@ export function isExecutableBlock(block: YBlock): boolean {
   })
 }
 
+export function isInputBlock(block: YBlock): boolean {
+  return switchBlockType(block, {
+    onPython: () => false,
+    onSQL: () => false,
+    onVisualization: () => false,
+    onInput: () => true,
+    onDropdownInput: () => true,
+    onWriteback: () => false,
+    onDateInput: () => true,
+    onRichText: () => false,
+    onFileUpload: () => false,
+    onDashboardHeader: () => false,
+    onPivotTable: () => false,
+  })
+}
+
 export function duplicateYText(text: Y.Text): Y.Text {
   const newText = new Y.Text()
   newText.insert(0, text.toString())
@@ -393,12 +417,25 @@ function getExecutedAt(block: YBlock, blocks: Y.Map<YBlock>): Date | null {
 function mustExecute(
   block: YBlock,
   blocks: Y.Map<YBlock>,
-  environmentStartedAt: Date | null
+  environmentStartedAt: Date | null,
+  skipDependencyCheck: boolean
 ): boolean {
   if (!isExecutableBlock(block)) {
     return false
   }
 
+  // We should always run input blocks if they have not been run yet
+  // even if skipDependencyCheck is true
+  if (skipDependencyCheck) {
+    const lastExecutedAt = getExecutedAt(block, blocks)
+    const lastExecutedAtIsAfterEnvironmentStartedAt =
+      lastExecutedAt === null ||
+      environmentStartedAt === null ||
+      dfns.isAfter(environmentStartedAt, lastExecutedAt)
+
+    return isInputBlock(block) && lastExecutedAtIsAfterEnvironmentStartedAt
+  }
+
   if (environmentStartedAt === null) {
     return true
   }
@@ -415,7 +452,8 @@ export function computeDepencyQueue(
   block: YBlock,
   layout: Y.Array<YBlockGroup>,
   blocks: Y.Map<YBlock>,
-  environmentStartedAt: Date | null
+  environmentStartedAt: Date | null,
+  skipDependencyCheck: boolean
 ): YBlock[] {
   const flatLayout = layout
     .toArray()
@@ -438,7 +476,7 @@ export function computeDepencyQueue(
 
   const blocksBefore = flatLayout.slice(0, blockIndex)
   const blocksBeforeToRun = blocksBefore.filter((block) =>
-    mustExecute(block, blocks, environmentStartedAt)
+    mustExecute(block, blocks, environmentStartedAt, skipDependencyCheck)
   )
 
   return blocksBeforeToRun

diff --git a/packages/editor/src/blocks/input.test.ts b/packages/editor/src/blocks/input.test.ts
@@ -4,7 +4,7 @@ import {
   BlockType,
   YBlock,
   getInputAttributes,
-  isInputBlock,
+  isTextInputBlock,
   getInputValueExecStatus,
   getInputVariableExecStatus,
   makeInputBlock,
@@ -20,7 +20,7 @@ describe('isYInputBlock', () => {
     const inputBlock = makeInputBlock('blockId', blocks)
     blocks.set('blockId', inputBlock)
 
-    expect(isInputBlock(inputBlock)).toBe(true)
+    expect(isTextInputBlock(inputBlock)).toBe(true)
   })
 
   it('should return false for non-Input blocks', () => {
@@ -29,7 +29,7 @@ describe('isYInputBlock', () => {
     const sqlBlock = makeSQLBlock('blockId', blocks)
     blocks.set('blockId', sqlBlock)
 
-    expect(isInputBlock(sqlBlock)).toBe(false)
+    expect(isTextInputBlock(sqlBlock)).toBe(false)
   })
 })
 

diff --git a/packages/editor/src/blocks/input.ts b/packages/editor/src/blocks/input.ts
@@ -30,7 +30,7 @@ export type InputBlock = BaseBlock<BlockType.Input> & {
   executedAt: string | null
 }
 
-export const isInputBlock = (
+export const isTextInputBlock = (
   block: YBlock
 ): block is Y.XmlElement<InputBlock> =>
   block.getAttribute('type') === BlockType.Input
@@ -234,7 +234,7 @@ export function getInputBlockExecStatus(
 function getAvailableInputVariable(
   blocks: Y.Map<YBlock>
 ): InputBlock['variable'] {
-  const inputBlocks = Array.from(blocks.values()).filter(isInputBlock)
+  const inputBlocks = Array.from(blocks.values()).filter(isTextInputBlock)
   const vars = new Set(
     inputBlocks.map((block) => block.getAttribute('variable')?.value)
   )
-Original file line number
+Diff line change
@@ Expand Up / @@ -65,6 +65,8 @@ export class InputObserver implements IInputObserver { @@
             status: 'idle',
           })
         }
+        this.executor.saveValue(block)
       }
       public async handleBlockEvent(
@@ Expand Down @@