Feat improve combine with effects batching

src/effector/__tests__/sample/order.test.ts

@@ -10,6 +10,7 @@ import {
   Store,
   Event,
   Effect,
+  StoreValue,
 } from 'effector'
 import {argumentHistory} from 'effector/fixtures'
 
@@ -443,6 +444,212 @@ describe('combine+sample cases', () => {
   })
 })
 
+describe('sample+combine+effects edge-cases', () => {
+  test('sample(combine,fx) + late samples', async () => {
+    const run = createEvent()
+
+    const $a = createStore(0)
+    const $b = createStore(0)
+
+    const refetch = createEvent()
+    const fetchFx = createEffect(() => null)
+
+    const $combine = combine({a: $a, b: $b})
+
+    const intermediateEvent = createEvent()
+
+    sample({
+      clock: run,
+      target: refetch,
+    })
+
+    sample({
+      clock: refetch,
+      source: $combine,
+      target: [intermediateEvent, fetchFx],
+    })
+
+    sample({
+      clock: run,
+      fn: () => 5,
+      target: $a,
+    })
+
+    sample({
+      clock: run,
+      fn: () => 10,
+      target: $b,
+    })
+
+    const fn = jest.fn()
+    watchAll(fn, [run, $a, $b, $combine, intermediateEvent, fetchFx, refetch])
+
+    fn(`## init complete`)
+
+    run()
+
+    expect(argumentHistory(fn)).toMatchInlineSnapshot(`
+      Array [
+        "$a: 0",
+        "$b: 0",
+        "$combine: {a:0,b:0}",
+        "## init complete",
+        "run: void",
+        "refetch: void",
+        "intermediateEvent: {a:0,b:0}",
+        "$a: 5",
+        "$combine: {a:5,b:0}",
+        "$b: 10",
+        "$combine: {a:5,b:10}",
+        "fetchFx: {a:5,b:10}",
+        "fetchFx.done: null",
+      ]
+    `)
+  })
+
+  test('sample(combine,fx) + late samples + intermediate', async () => {
+    const run = createEvent()
+
+    const $a = createStore(0)
+    const $b = createStore(0)
+
+    const refetch = createEvent()
+    const fetchFx = createEffect(() => null)
+
+    const $combine = combine({a: $a, b: $b})
+
+    const intermediateEvent = createEvent<StoreValue<typeof $combine>>()
+
+    sample({
+      clock: run,
+      target: refetch,
+    })
+
+    sample({
+      clock: refetch,
+      source: $combine,
+      target: [intermediateEvent, fetchFx],
+    })
+
+    sample({
+      clock: run,
+      fn: () => 5,
+      target: $a,
+    })
+
+    sample({
+      clock: run,
+      fn: () => 10,
+      target: $b,
+    })
+
+    sample({
+      clock: intermediateEvent,
+      fn: ({a, b}) => a + b,
+      target: $a,
+    })
+
+    const fn = jest.fn()
+    watchAll(fn, [run, $a, $b, $combine, intermediateEvent, fetchFx, refetch])
+
+    fn(`## init complete`)
+
+    run()
+
+    expect(argumentHistory(fn)).toMatchInlineSnapshot(`
+      Array [
+        "$a: 0",
+        "$b: 0",
+        "$combine: {a:0,b:0}",
+        "## init complete",
+        "run: void",
+        "refetch: void",
+        "intermediateEvent: {a:0,b:0}",
+        "$a: 5",
+        "$combine: {a:5,b:0}",
+        "$b: 10",
+        "$combine: {a:5,b:10}",
+        "$a: 0",
+        "$combine: {a:0,b:10}",
+        "fetchFx: {a:0,b:10}",
+        "fetchFx.done: null",
+      ]
+    `)
+  })
+
+  test('greedy: sample(combine,fx) + late samples + intermediate', async () => {
+    const run = createEvent()
+
+    const $a = createStore(0)
+    const $b = createStore(0)
+
+    const refetch = createEvent()
+    const fetchFx = createEffect(() => null)
+
+    const $combine = combine({a: $a, b: $b})
+
+    const intermediateEvent = createEvent<StoreValue<typeof $combine>>()
+
+    sample({
+      clock: run,
+      target: refetch,
+    })
+
+    sample({
+      clock: refetch,
+      source: $combine,
+      target: [intermediateEvent, fetchFx],
+    })
+
+    sample({
+      clock: run,
+      fn: () => 5,
+      target: $a,
+      greedy: true,
+    })
+
+    sample({
+      clock: run,
+      fn: () => 10,
+      target: $b,
+      greedy: true,
+    })
+
+    sample({
+      clock: intermediateEvent,
+      fn: ({a, b}) => a + b,
+      target: $a,
+      greedy: true,
+    })
+
+    const fn = jest.fn()
+    watchAll(fn, [run, $a, $b, $combine, intermediateEvent, fetchFx, refetch])
+
+    fn(`## init complete`)
+
+    run()
+
+    expect(argumentHistory(fn)).toMatchInlineSnapshot(`
+      Array [
+        "$a: 0",
+        "$b: 0",
+        "$combine: {a:0,b:0}",
+        "## init complete",
+        "run: void",
+        "$a: 5",
+        "$b: 10",
+        "$combine: {a:5,b:10}",
+        "refetch: void",
+        "intermediateEvent: {a:5,b:10}",
+        "$a: 15",
+        "$combine: {a:15,b:10}",
+        "fetchFx: {a:15,b:10}",
+        "fetchFx.done: null",
+      ]
+    `)
+  })
+})
+
 function watchAll(
   fn: jest.Mock<any, any>,
   units: Array<Store<any> | Event<any> | Effect<any, any>>,

src/effector/sample.ts

@@ -171,27 +171,60 @@ export const createSampling = (
     clockState,
     method,
   )
-  const jointNode = createLinkNode(
+
+  const sharedSeq = () => [
+    applyTemplate('sampleSourceLoader'),
+    mov({from: STACK, target: clockState}),
+    ...readAndFilter(hasSource),
+    read(sourceRef, true, batched),
+    ...filterNodes,
+    read(clockState),
+    filterType === 'fn' && userFnCall((src, _, {a}) => filter(src, a), true),
+  ]
+
+  const [units, effects] = splitTargetGroups(target)
+
+  if (units.length > 0) {
+    const jointNode = createLinkNode(
+      // @ts-expect-error
+      clock,
+      units,
+      [
+        ...sharedSeq(),
+        fn && userFnCall(callStackAReg),
+        applyTemplate('sampleSourceUpward', isUpward),
+      ],
+      method,
+      fn,
+    )
     // @ts-expect-error
-    clock,
-    target,
-    [
-      applyTemplate('sampleSourceLoader'),
-      mov({from: STACK, target: clockState}),
-      ...readAndFilter(hasSource),
-      read(sourceRef, true, batched),
-      ...filterNodes,
-      read(clockState),
-      filterType === 'fn' && userFnCall((src, _, {a}) => filter(src, a), true),
-      fn && userFnCall(callStackAReg),
-      applyTemplate('sampleSourceUpward', isUpward),
-    ],
-    method,
-    fn,
-  )
-  // @ts-expect-error
-  own(source, [jointNode])
-  Object.assign(jointNode.meta, metadata, {joint: true})
+    own(source, [jointNode])
+    Object.assign(jointNode.meta, metadata, {joint: true})
+  }
+
+  if (effects.length > 0) {
+    const sourceReader = read(sourceRef, true)
+
+    sourceReader.order = {priority: 'effect'}
+
+    const jointNode = createLinkNode(
+      // @ts-expect-error
+      clock,
+      effects,
+      [
+        ...sharedSeq(),
+        // effect-targets should read sourceRef at their own
+        sourceReader,
+        fn && userFnCall(callStackAReg),
+        applyTemplate('sampleSourceUpward', isUpward),
+      ],
+      method,
+      fn,
+    )
+    // @ts-expect-error
+    own(source, [jointNode])
+    Object.assign(jointNode.meta, metadata, {joint: true})
+  }
   return target
 }
 
@@ -225,3 +258,18 @@ const syncSourceState = (
   applyTemplate('sampleSource', hasSource, sourceRef, clockState)
   return [sourceRef, hasSource] as const
 }
+
+const splitTargetGroups = (target: DataCarrier | DataCarrier[]) => {
+  const units: DataCarrier[] = []
+  const effects: DataCarrier[] = []
+
+  ;(Array.isArray(target) ? target : [target]).forEach(t => {
+    if (is.effect(t)) {
+      effects.push(t)
+    } else {
+      units.push(t)
+    }
+  })
+
+  return [units, effects] as const
+}