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

Unnecessary implicit conversion while using experimental catseffect AsyncValue #1041

Open
roman0x58 opened this issue Aug 11, 2023 · 4 comments
Assignees

Comments

@roman0x58
Copy link

Hello there, I'm trying to eliminate the need for the Future with the functionality described here: #987. However, I'm encountering a lot of ambiguity errors in places where F[X] is used. The implicit conversion in the AsyncValue object is attempting to convert the types to LeafAction. Please see the errors above for more details. Do you have any advice on how to address this ambiguity? Thanks!


[error] 133 |        parentPrice <- fs2.Stream.eval(collection(ctx).findById(parentId).compile.toList.map(_.headOption))
[error]     |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |Found:    F[List[O2]]
[error]     |Required: ?{ map: ? }
[error]     |
[error]     |where:    F  is a type in class PriceDictionary with bounds <: [_] =>> Any
[error]     |          O2 is a type variable with constraint >: model.TreeElement[model.PriceItem]
[error]     |
[error]     |Note that implicit extension methods cannot be applied because they are ambiguous;
[error]     |both method asyncAction in object AsyncValue and method toFunctorOps in trait ToFunctorOps provide an extension method `map` on (fs2.Stream.CompileOps[[x] =>> F[x], [x] =>> F[x], O2]#toList : => F[List[O2]])
[error] 86 |    (for {
[error] 87 |      user <- c.ctx.om.mongo.users
[error] 88 |        .findOne(or(equal("email", email), equal("login", email)))
[error] 89 |        .compile
[error] 90 |        .toList
[error] 91 |        .map(_.headOption)
[error] 92 |    } yield user match {
[error] 93 |      case Some(u) => checkAuthContext(u, c.arg(Context))(process(_))
[error] 94 |      case _ => failForm[UserPassport]("email", messages("om.auth.violation.notfound"))
[error] 95 |    }).flatten
[error]    |    ^
[error]    |value flatten is not a member of sangria.schema.LeafAction[Nothing, F[auth.UserPassport]], but could be made available as an extension method.
@yanns
Copy link
Contributor

yanns commented Aug 14, 2023

Thanks for the report.
Indeed, the scala compiler seems to transform the F[_] into a LeafAction too early.
Maybe I'll need to push the implicit conversion into a trait to give it less priority.
I can try that later.

I'm not sure if we would have the same issue when using Future.

For the moment, as a workaround, we can add type hints.
Or better, you could extract your logic into another function returning the F[_] and only call this function from sangria. That way, you would make sure that your business logic is not coupled with sangria.

@yanns yanns self-assigned this Aug 14, 2023
@roman0x58
Copy link
Author

Thanks for the tips, @yanns! Decoupling sangria and the business layers indeed seems like the best option. However, implementing this would require a significant amount of work due to our solid codebase. For now, I think I'll stick with the cats dispatcher.

yanns added a commit that referenced this issue Nov 7, 2024
@yanns
Copy link
Contributor

yanns commented Nov 7, 2024

I think I have a solution for that.
But before, I'm trying to reproduce your issue, but I could not. Here is my try: #1156
Maybe you could help here?

@roman0x58
Copy link
Author

I think I have a solution for that. But before, I'm trying to reproduce your issue, but I could not. Here is my try: #1156 Maybe you could help here?

I replied to your pull request. I think the second case with flatten leads to the first

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

2 participants