Skip to content

Add WithCookies overloads for IEnumerable key-value pairs.#866

Open
tranb3r wants to merge 1 commit intotmenier:devfrom
tranb3r:fix-reflection
Open

Add WithCookies overloads for IEnumerable key-value pairs.#866
tranb3r wants to merge 1 commit intotmenier:devfrom
tranb3r:fix-reflection

Conversation

@tranb3r
Copy link
Copy Markdown
Contributor

@tranb3r tranb3r commented Jan 2, 2026

Description

This PR proposes a minor change of how cookies are handled internally when using WithCookies, with the goal of improving performance, reducing unnecessary conversions, and improving compatibility with NativeAOT scenarios.
I did not open a separate issue, so I’m documenting the full motivation and design discussion here.

Avoid unnecessary reflection and double conversions

Currently, when WithCookies is called with an object, Flurl relies on ToKeyValuePairs, which uses reflection extensively.

In practice, this leads to an inefficient flow:

  • Flurl code already has cookies as an IEnumerable<KeyValuePair<string, string> in ApplyCookieJar.
  • This collection is converted into an object
  • ToKeyValuePairs is then called, using reflection to convert it back into a collection

This double conversion is unnecessary and adds avoidable reflection overhead.
The reflection cost is especially unfortunate given that the data is already in a strongly typed form.

This PR removes that inefficiency by:

  • Avoiding the object-based path when a typed collection is already available
  • Introducing a type-safe extension that can be used both internally by Flurl and externally by library users

NativeAOT compatibility (e.g. .NET MAUI)

The current ToKeyValuePairs implementation does not work in some NativeAOT scenarios (e.g. .NET MAUI with NativeAOT enabled).

Because NativeAOT severely restricts or eliminates runtime reflection, this code path fails at runtime.
While NativeAOT is still experimental, it is clearly becoming more relevant, and reflection-heavy code should be avoided where possible.

I don’t have a minimal repro project to share, but I encountered this issue in a real scenario:

  • A server sets a cookie in the response
  • A redirect occurs
  • Flurl applies cookies using the current reflection-based path
  • The application fails under NativeAOT

Rather than trying to “fix” ToKeyValuePairs for NativeAOT (which would be complex and fragile), this PR takes a simpler and more robust approach: avoid calling the untyped, reflection-based extension altogether when a typed alternative can be used.

Public extension vs. private helper

This PR introduces a public, type-safe extension method.

An alternative would be to keep this logic private and only use it internally (e.g. in ApplyCookieJar).
However, I believe exposing a typed extension is beneficial:

  • It gives Flurl users a clear, safe, and efficient API
  • It avoids encouraging the object-based overload in new code
  • It aligns with the direction of minimizing reflection-heavy APIs

That said, I’m open to discussion on this point if you’d prefer to keep the surface area smaller.

Summary

  • Removes unnecessary object → reflection → collection conversions
  • Improves performance by avoiding reflection where possible
  • Fixes incompatibility with NativeAOT scenarios
  • Introduces a type-safe API that can be reused internally and externally

Feedback and suggestions are very welcome — especially regarding the public API choice.
Thanks for considering this change!

@tranb3r
Copy link
Copy Markdown
Contributor Author

tranb3r commented Jan 7, 2026

@tmenier Hi Todd,
I was wondering whether you’re still actively working on Flurl and if a new release is planned in the coming weeks/months.
Also, if you have a moment, could you rerun the tests on this PR? I can also push an empty commit if that’s easier—just let me know.
Thanks!

@tmenier
Copy link
Copy Markdown
Owner

tmenier commented Jan 9, 2026

Sure thing. I guess you could say it's in maintenance mode at best right now, I just haven't had the time/energy to work on it in a while. I'll look at this one and try and release it soonish, though I'm going on vacation in a couple days so I may not get to it until I get back.

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