Using flow in combination with Effect.reduce incorrectly reuses past accumulator

[taylorjohnwood]

What version of Effect is running?

3.12.1

What steps can reproduce the bug?

This test demonstrates the bug clearly

class MyNumberObject {
  number: number;
  constructor(number: number) {
    this.number = number;
  }

  plus(num: number): MyNumberObject {
    this.number += num;
    return this;
  }
}

it("Bug in effect reduce when using flow", () => {
  const sumUpNumbersBad = flow(
    (numbers: number[]) => numbers,
    Effect.reduce(new MyNumberObject(0), (acc, num, i) => {
      return Effect.succeed(acc.plus(num));
    }),
  );
  console.log(
    "sumUpNumbersBad result #1",
    sumUpNumbersBad([1, 2, 3, 4, 5]).pipe(Effect.runSync),
  );
  console.log(
    "sumUpNumbersBad result #2",
    sumUpNumbersBad([1, 10]).pipe(Effect.runSync),
  );

  const sumUpNumbersGood = (numbers: number[]) =>
    pipe(
      numbers,
      Effect.reduce(new MyNumberObject(0), (acc, num) =>
        Effect.succeed(acc.plus(num)),
      ),
    );
  console.log(
    "sumUpNumbersGood result #1",
    sumUpNumbersGood([1, 2, 3, 4, 5]).pipe(Effect.runSync),
  );
  console.log(
    "sumUpNumbersGood result #2",
    sumUpNumbersGood([1, 2, 3, 4, 5]).pipe(Effect.runSync),
  );
});

What is the expected behavior?

I would expect this test to output

sumUpNumbersBad result #1 MyNumberObject { number: 15 }
sumUpNumbersBad result #2 MyNumberObject { number: 11 }
sumUpNumbersGood result #1 MyNumberObject { number: 15 }
sumUpNumbersGood result #2 MyNumberObject { number: 11 }

What do you see instead?

Instead I see an incorrect result for sumUpNumbersBad result 2

sumUpNumbersBad result #1 MyNumberObject { number: 15 }
sumUpNumbersBad result #2 MyNumberObject { number: 26 }
sumUpNumbersGood result #1 MyNumberObject { number: 15 }
sumUpNumbersGood result #2 MyNumberObject { number: 11 }

Additional information

Potential issues with flow are hinted at in the docs here
https://effect.website/docs/code-style/guidelines/#avoid-tacit-usage

But I still believe that this behavior is unacceptable and could impact many users of Effect.

[tim-smart]

The issue is that you are mutating the same variable on every run. If you want a new instance each time then you need to wrap the creation with something like Effect.sync first.

The "good" versions works because it is wrapped with a closure.

[taylorjohnwood]

Thanks for getting back to me so quickly @tim-smart .

I agree that somewhere along the line a reference to the object created atnew MyNumberObject(0) is being kept and reused, but I doubt that is considered "working as intended"?

Every time the sumUpNumbersBad function is invoked the Effect.reduce function is invoked too. Every time Effect.reduce is invoked the zero starting value for the accumulator should be re-instantiated and not reused.

[tim-smart]

If you are supplying the same reference as the initial value every time then it is working as intended.

If you want to work with mutable state you have to be wary of the foot-guns.

In this case you have to ensure a new instance of the number class is created on each run of the effect - that is something the developer is responsible for :)

We have been evaluating the use of lazy evaluation for the initial argument to help mitigate this, but for now it is up to you.

[taylorjohnwood]

Thanks for stepping through it all with me, after reading your replies I understand the issue.

A potential solution would be this

const sumUpNumbers = flow<[number[]], Effect.Effect<MyNumberObject>>(
    (numbers) =>
      Effect.suspend(() =>
        Effect.reduce(numbers,new MyNumberObject(0), (acc, num) => {
          return Effect.succeed(acc.plus(num));
        }),
      ),
  );

Perhaps a function like this could be a nice little bit of 'sugar'

const sumUpNumbers = flow<[number[]], Effect.Effect<MyNumberObject>>(
      Effect.suspendFlow((numbers) =>
        Effect.reduce(numbers, new MyNumberObject(0), (acc, num) => {
          return Effect.succeed(acc.plus(num));
        }),
      ),
  );

---

(As an aside I cant see what the difference between Effect.suspend and Effect.sync are in this case)