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

System.Interactive.Async dependency is problematic (transitive) #329

Closed
mgravell opened this issue Jun 14, 2019 · 7 comments
Closed

System.Interactive.Async dependency is problematic (transitive) #329

mgravell opened this issue Jun 14, 2019 · 7 comments

Comments

@mgravell
Copy link
Contributor

Currently, the lib takes a transitive dependency on System.Interactive.Async v3.2.0" via Grpc.Core.

This is problematic because it defines IAsyncEnumerable<T> etc, which means that anyone taking a dep on this lib cannot use async-enumerable:

error CS0433: The type 'IAsyncEnumerable' exists in both 'System.Interactive.Async, Version=3.2.0.0, Culture=neutral, PublicKeyToken=94bc3704cddfc263' and 'System.Runtime, Version=4.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'

The ideal fix here would be if there was a preview of Grpc.Core that took a dep on v4.0.0 (preview); as I understand it, v4.0.0 unifies the types to avoid this problem - but of course v4.0.0 is preview as long as core 3 is preview; so it would need to be a preview of Grpc.Core too, etc. But that's probably all correct.

The main point of this issue is to make it explicit that it would be a bad idea to release Grpc-Dotnet with it taking a transitive dependency that breaks async enumerables.

Of course, if we can co-ordinate with Grpc.Core to get a preview build that works correctly with them, that's even better - it would allow a full test of more scenarios, and would prevent the risk of accidentally deploying with a dangerously broken API.

@mgravell mgravell changed the title System.Interactive.Async preview is problematic (transitive) System.Interactive.Async dependency is problematic (transitive) Jun 14, 2019
@mgravell
Copy link
Contributor Author

mgravell commented Jun 14, 2019

In case you're wondering about the context here: my code-first stuff implements API re-shaping. The following already works:

    [ServiceContract(Name = "Greet.Greeter")]
    public interface IGreeter
    {
        ValueTask<HelloReply> SayHelloAsync(HelloRequest request);
    }

but I'd ideally like it to also work to detect client/server/duplex streaming APIs via IAsyncEnumerable<T>, i.e.

IAsyncEnumerable<SomeReply> FullDuplex(IAsyncEnumerable<SomeRequest> whatever);

I can't begin to look at this as long as the transitive dependency problem is there.

(the key point of API re-shaping is that it allows a single interface to be shared at both client and server, with the server implementing the interface, and the client getting a proxy; i.e. my server here is a class SomeServer : IGreeter)

@JamesNK
Copy link
Member

JamesNK commented Jun 14, 2019

I've already brought this up here - grpc/grpc#19059

You should add a comment that this is important to resolve.

@mgravell
Copy link
Contributor Author

Awesome, thanks. Chimed in.

@JamesNK
Copy link
Member

JamesNK commented Jun 14, 2019

By the way you can work around this. It's not user friendly, and not a good long term solution but it will unblock you. I'll look up a sample I wrote.

@JamesNK
Copy link
Member

JamesNK commented Jun 14, 2019

Put this in your csproj:

  <!--
    System.Interactive.Async contains IAsyncEnumerable<T> that conflicts with .NET Core version
    Give assembly an alias so .NET Core version can be referenced in C#
  -->
  <Target Name="ChangeAliasesOfReactiveExtensions" BeforeTargets="FindReferenceAssembliesForReferences;ResolveReferences">
    <ItemGroup>
      <ReferencePath Condition="'%(FileName)' == 'System.Interactive.Async'">
        <Aliases>ix</Aliases>
      </ReferencePath>
    </ItemGroup>
  </Target>

@mgravell
Copy link
Contributor Author

Oren sent me.that via twitter. I didn't realise it was yours, but nice: thanks!

@JamesNK
Copy link
Member

JamesNK commented Jun 17, 2019

Covered by grpc/grpc#19059

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