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

Tuple Support for Headers #536

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Conversation

eldonferranpol
Copy link

Headers to allow the passing of tuples along these lines:

request.WithHeaders(("Accept", "application/json"), ("host", "localhost"), ("etc", "etc"))

@tmenier
Copy link
Owner

tmenier commented Jul 30, 2020

Thanks. I like the idea. There's a few things I'd like to see before accepting this though:

  1. System.ValueTuple is already included on new platforms, such as .NET Framework 4.7 and above. I would like the NuGet dependency in the csproj file to be conditional to only those platforms that need it. I believe this can be relied upon as a guide: https://blog.monstuff.com/archives/2017/03/valuetuple-availability.html (A lot of people use Flurl in Xamarin and Blazor apps and they WILL notice an unnecessary dependency.)

  2. For consistency, it would be great if tuples were supported everywhere throughout Flurl where parsing objects to name-value pairs is supported. This includes query strings, multipart, and cookies. Maybe others. If you search the code for ToKeyValuePairs it should guide you to those things.

  3. WithHeaders (and other methods that get new overloads per # 2) should have equivalent extension methods for Url, Uri, and string, in support of the various ways you can fluently string together statements. Flurl.Http.CodeGen should be modified and run to take care of those.

@eldonferranpol
Copy link
Author

eldonferranpol commented Jul 30, 2020 via email

@tmenier
Copy link
Owner

tmenier commented Jul 30, 2020

Yes, funny you mention that about RespondWith. I tried to write a test involving multiple Set-Cookie headers and that's not so easy - tuples would work great. I've actually started modifying some of the internals of ToKeyValuePair to make special accommodations for IEnumerable<ValueTuple>. So we might end up working on this together. Can you hold off a few days?

Another thing I should mention is that this wouldn't get released before 3.0, and as part of that I may re-evaluate which platforms/versions I want to continue to support going forward. That could have some impact on if/how to include System.ValueTuple as well. Another reason to consider holding off a bit - I should have an answer on that soon.

@eldonferranpol
Copy link
Author

eldonferranpol commented Jul 31, 2020 via email

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

Successfully merging this pull request may close these issues.

2 participants