Fix frequent combine updates on first allSettled call (#984)

* Add test for combine allSettled retriggers

(cherry picked from commit 2358204)

* Add more tests for retriggers

(cherry picked from commit c608a98)

* Add another test for combine retriggering

(cherry picked from commit eb84a1665e4a98d336e09a7df6a49f8cd45712ab)

* Fix combine retriggering

* Skip variables initialization in happy path in initRefInScope

src/effector/__tests__/__snapshots__/index.test.ts.snap

@@ -231,7 +231,8 @@ exports[`showcase: event foo 1`] = `
                 mov: a store,
                 mov: value store,
                 mov: value store,
-                mov: store stack
+                mov: store stack,
+                mov: store b
               ],
               next: [
                 Node {
@@ -319,7 +320,8 @@ exports[`showcase: store a 1`] = `
         mov: a store,
         mov: value store,
         mov: value store,
-        mov: store stack
+        mov: store stack,
+        mov: store b
       ],
       next: [
         Node {

src/effector/__tests__/combine.test.ts

@@ -6,6 +6,8 @@ import {
   createEvent,
   EffectResult,
   createDomain,
+  fork,
+  allSettled,
 } from 'effector'
 import {argumentHistory} from 'effector/fixtures'
 
@@ -340,7 +342,10 @@ describe('don`t reuse values from user', () => {
     const triggerB = createEvent()
     const foo = createStore(0)
     const bar = createStore(0).on(triggerB, x => x + 10)
-    const combined = combine({foo, bar})
+    const combined = createStore({foo: 0, bar: 0}).on(
+      combine({foo, bar}),
+      (_, x) => x,
+    )
     sample({
       clock: triggerA,
       source: combined,
@@ -366,7 +371,10 @@ describe('don`t reuse values from user', () => {
     const triggerB = createEvent()
     const foo = createStore(0)
     const bar = createStore(0).on(triggerB, x => x + 10)
-    const combined = combine({foo, bar})
+    const combined = createStore({foo: 0, bar: 0}).on(
+      combine({foo, bar}),
+      (_, x) => x,
+    )
     combined.on(triggerA, ({foo, bar}) => ({
       foo: foo + 1,
       bar: bar + 1,
@@ -383,3 +391,45 @@ describe('don`t reuse values from user', () => {
     expect(combined.getState()).toEqual({foo: 0, bar: 20})
   })
 })
+
+describe('fn retriggers', () => {
+  test('dont retrigger combine fn on allSettled calls', async () => {
+    const fn = jest.fn()
+    const inc = createEvent()
+    const $a = createStore(0).on(inc, a => a + 1)
+    const $comb = combine($a, a => {
+      fn(a)
+      return a
+    })
+
+    const scope = fork({values: [[$a, 10]]})
+    await allSettled(inc, {scope})
+    expect(argumentHistory(fn)).toEqual([0, 11])
+  })
+  test('dont retrigger combine fn on getState calls', () => {
+    const fn = jest.fn()
+    const $a = createStore(0)
+    const $comb = combine($a, a => {
+      fn(a)
+      return a
+    })
+
+    const scope = fork({values: [[$a, 10]]})
+    scope.getState($comb)
+    expect(argumentHistory(fn)).toEqual([0, 10])
+  })
+  test('dont retrigger combine fn on getState + allSettled calls', async () => {
+    const fn = jest.fn()
+    const inc = createEvent()
+    const $a = createStore(0).on(inc, a => a + 1)
+    const $comb = combine($a, a => {
+      fn(a)
+      return a
+    })
+
+    const scope = fork({values: [[$a, 10]]})
+    scope.getState($comb)
+    await allSettled(inc, {scope})
+    expect(argumentHistory(fn)).toEqual([0, 10, 11])
+  })
+})

src/effector/__tests__/effector.test.ts

@@ -235,7 +235,7 @@ test('no dull updates', () => {
   expect(fn2).toHaveBeenCalledTimes(5)
   expect(fn3).toHaveBeenCalledTimes(5)
 })
-test('no dull updates with fork', async () => {
+test('no dull updates with fork and map', async () => {
   const store = createStore(false)
   const e1 = createEvent<boolean>()
   const e2 = createEvent<boolean>()
@@ -286,6 +286,57 @@ test('no dull updates with fork', async () => {
   expect(fn3).toHaveBeenCalledTimes(5)
 })
 
+test('no dull updates with fork and combine', async () => {
+  const store = createStore(false)
+  const e1 = createEvent<boolean>()
+  const e2 = createEvent<boolean>()
+  const fn1 = jest.fn()
+  const fn2 = jest.fn()
+  const fn3 = jest.fn()
+  store.watch(fn1)
+  store.on(e1, (_, payload): boolean => payload)
+  store.on(e2, (_, p) => _ === p)
+  const nextStore = combine(store, x => (fn2(x), x))
+  nextStore.watch(fn3)
+  store.watch(() => {})
+  const scope = fork()
+  await allSettled(e1, {params: false, scope})
+  await allSettled(e1, {params: true, scope})
+  await allSettled(e1, {params: false, scope})
+  await allSettled(e2, {params: false, scope})
+  await allSettled(e2, {params: false, scope})
+  expect(argumentHistory(fn1)).toMatchInlineSnapshot(`
+    Array [
+      false,
+      true,
+      false,
+      true,
+      false,
+    ]
+  `)
+  expect(argumentHistory(fn2)).toMatchInlineSnapshot(`
+    Array [
+      false,
+      true,
+      false,
+      true,
+      false,
+    ]
+  `)
+  expect(argumentHistory(fn3)).toMatchInlineSnapshot(`
+    Array [
+      false,
+      true,
+      false,
+      true,
+      false,
+    ]
+  `)
+  expect(fn1).toHaveBeenCalledTimes(5)
+  expect(fn2).toHaveBeenCalledTimes(5)
+  expect(fn3).toHaveBeenCalledTimes(5)
+})
+
 test('smoke', async () => {
   const used = jest.fn(x => Promise.resolve(x))
   const usedDone = jest.fn(x => Promise.resolve(x))

src/effector/combine.ts

@@ -109,6 +109,17 @@ const storeCombination = (
    * (thats why order has no "barrierID" field, which assume batching)
    **/
   rawShapeReader.order = {priority: 'barrier'}
+  /**
+   * Soft store reading is required for
+   * setting target store as inited in scope
+   * for preventing retriggering issues
+   **/
+  const softReader = mov({
+    store: storeStateRef,
+    to: 'b',
+    priority: 'read',
+  })
+  softReader.data.softRead = true
   const node = [
     calc((upd, _, stack) => {
       if (stack.scope && !stack.scope.reg[rawShape.id]) {
@@ -138,6 +149,7 @@ const storeCombination = (
     }),
     read(rawShape, true),
     fn && userFnCall(),
+    softReader,
   ]
   forIn(obj, (child: Store<any> | any, key) => {
     if (!is.store(child)) {

src/effector/kernel.ts

@@ -483,13 +483,13 @@ export const initRefInScope = (
   softRead?: boolean,
 ) => {
   const refsMap = scope.reg
+  if (refsMap[sourceRef.id]) return
   const sid = sourceRef.sid
   const serialize = sourceRef?.meta?.serialize
   const parser =
     scope.fromSerialize && serialize !== 'ignore'
       ? serialize?.read || noopParser
       : noopParser
-  if (refsMap[sourceRef.id]) return
   const ref: StateRef = {
     id: sourceRef.id,
     current: sourceRef.current,