Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(very breaking) change in Future instances? #4617

Closed
yurique opened this issue Jun 11, 2024 · 5 comments
Closed

(very breaking) change in Future instances? #4617

yurique opened this issue Jun 11, 2024 · 5 comments

Comments

@yurique
Copy link

yurique commented Jun 11, 2024

The following code works differently with cats 2.10.0 and cats 2.11.0+

def test(i: Int): Future[Unit] = 
  for {
    _ <- Future { println(s"start $i") } 
    _ <- futureSleep(100.millis) // just to avoid any timing issues, reproduces without it as well
    _ <- Future { println(s"end $i") } 
  } yield ()

List(1, 2, 3).traverse(test)

With 2.10.0 it prints (https://scastie.scala-lang.org/RYQcjXExR4m0NQQBHVo25w):

start 1
end 1
start 2
end 2
start 3
end 3

With 2.11.0+ (https://scastie.scala-lang.org/F21nFlI1Tl2bYm271o50ng):

start 1
start 2
start 3
end 1
end 2
end 3
@mpilquist
Copy link
Member

Note the 2.11 behavior matches that of Future.traverse (https://scastie.scala-lang.org/aU1EoSBnQu2rbWiXtEbcsQ)

@yurique
Copy link
Author

yurique commented Jun 11, 2024

Right, it does 🤔.
But it doesn't match the traverse with IO: https://scastie.scala-lang.org/1JfGkbQOSTicrUyPaERQhw

Regardless, it still looks like a very breaking change.

@armanbilge
Copy link
Member

it still looks like a very breaking change

looks like, but is not. See previous discussion:

@johnynek
Copy link
Contributor

The problem is Future, and hence this function test: Int => Future[Unit] is not pure.

When you call test a side effect immediately starts. That is not true of the IO version. When you are working with Future you are working with an impure type and we don't limit the traverse implementation of data types to try to work that.

A PR was created where a lot of energy was spent to optimize traverse implementations: #4498 -- I think it is very likely the change you are seeing is due to that.

I guess what is happening is that since Future has a StackSafeMonad, it is traversing directly (which is what the standard library does), but that is triggering the side-effects in a different order.

@yurique
Copy link
Author

yurique commented Jun 11, 2024

looks like, but is not

It depends on how you look at it (or how much code you need to fix) 😆.


Thanks for chiming in, everyone! I'll close this one and will go hunting down my traverses :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants