-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Querystring building APIs for Blazor #34115
Comments
Tagging @dotnet/aspnet-blazor-eng @davidfowl for opinions. |
Just noticed @MackinnonBuck isn't in the group I tagged above, so mentioning him for any opinions too. |
@SteveSandersonMS I would separate things in three cases:
Adding a parameter is the "simplest" and can "relatively easy" be done without any special primitive (just raw string manipulation). (I'm not suggesting we do this). Removing and updating parameters is trickier because you need to know the query string structure. One potential way to go about this (I don't know if we already do) is to expose our One thing to note here, is that I don't think we have a problem when you are generating lots of links. ASP.NET Core link generation isn't allocation free and we don't think there's a compelling need to optimize it further. Its nice if we can have more "optimal" APIs, but I don't necessarily think we need to jump into it. I would suggest we avoid this problem at all or provide simple APIs with a basic implementation over ultra-optimizing things from the beginning. |
I'm open to the idea that we do nothing (that's what I wrote up as design 1 above) but there are some real and specific problems there for WebAssembly developers. We will need to have some specific recommendations about what APIs people should use, and how they should use them.
Understood, but I wouldn't generally take ASP.NET Core as a guide to the perf characteristics and requirements for Blazor. An interactive UI that may refresh tens of times per second and that has a degraded UX if GC happens can involve making some different tradeoffs to an HTTP server. I'm not disagreeing with your view that we may be able to avoid implementing something new here, but I think we should find justifications based on its own scenarios. Hope that sounds reasonable! |
Agree, at the same time, a traditional server handles hundreds of thousands of requests per second. I understand the potential concerns about UX experience in extreme cases, I'm suggesting that they are "extreme" and likely not something we necessarily need to solve ourselves. People with these extreme needs can come up with solutions that work for their needs on their own in the same way they do in other areas. For most people, an API that uses a StringBuilder or the ArrayPool internally to build the query will likely do the trick. |
I know - perf is important everywhere! I'm not disputing your conclusion, but just trying to indicate why I think there are grounds for making different tradeoffs sometimes. In this case it's to do with Blazor being more focused on lag, and ASP.NET Core being more focused on throughput. Anyway, that aside, we can't completely do nothing here. We have to provide some APIs (or recommendations) that are valid for WebAssembly. I'd love to get some perspectives from others on the team too, but @javiercn if you want to chat about options here just ping me whenever you want. |
API proposalI've boiled this down to what I think are the minimum reasonable scenarios:
These three cases naturally lead to three different method overloads, and since they are mainly focused on updating the current URL, they make perfectly good sense to hang on Rather than optimize excessively for perf at this stage (e.g., as @javiercn argues above) with the whole "builder" pattern to avoid allocations, I propose the following overloads. Note that there's already Public APIsnavigationManager.UriWithQueryParameter<T>(string name, T value)
navigationManager.UriWithQueryParameters(IReadOnlyDictionary<string, object> parameters)
navigationManager.UriWithQueryParameters(string uri, IReadOnlyDictionary<string, object> parameters)
For clarity, these can actually be extension methods that live in Non-goalsI no longer thing it's necessary or valuable to:
|
@pranavkm Hey API review boss, it it valid to "API review" a proposal for forthcoming API, or do we only review things after they are implemented? |
Is it then possible to set a query parameter to
What exactly is the Would it be possible to add / update multiple values for the same query parameter name? |
@campersau - thanks for commenting! I've edited to clarify that |
We certainly could support |
@MackinnonBuck Just in case you get to this before we next talk, the infrastructure I was mentioning that will be helpful here is |
Done in #34813. |
For API review: Here is the API surface diff after the changes in #34813 namespace Microsoft.AspNetCore.Components
{
+ public static class NavigationManagerExtensions
+ {
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, string? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, bool value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, bool? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, DateTime value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, DateTime? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, decimal value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, decimal? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, double value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, double? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, float value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, float? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, Guid value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, Guid? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, int value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, int? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, long value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, long? value)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<string?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<bool> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<bool?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<DateTime> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<DateTime?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<decimal> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<decimal?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<double> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<double?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<float> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<float?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<Guid> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<Guid?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<int> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<int?> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<long> values)
+ public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<long?> values)
+ public static string UriWithQueryParameters(this NavigationManager navigationManager, IReadOnlyDictionary<string, object?> parameters)
+ public static string UriWithQueryParameters(this NavigationManager navigationManager, string uri, IReadOnlyDictionary<string, object?> parameters)
+ }
} |
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
API review:
We also decided to remove all the - public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<string?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<bool> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<bool?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<DateTime> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<DateTime?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<decimal> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<decimal?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<double> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<double?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<float> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<float?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<Guid> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<Guid?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<int> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<int?> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<long> values)
- public static string UriWithQueryParameter(this NavigationManager navigationManager, string name, IEnumerable<long?> values) |
As part of #33338, I'm considering what (if anything) we should do to help people generate URLs with particular querystrings from Blazor components.
Scenarios
In both cases, it's worth thinking of these two sub-cases:
a. If you're navigating to a different
@page
component, or if your component completely owns and controls the whole URL, then you want to discard any other query parameters.b. If you're just updating query parameter(s) while staying on the same page, and unrelated components are also storing state in the querystring, you need to retain the unrelated parameters.
In other words, are you also retaining the path part of the URL, and are you retaining unrelated query params?
Sample for case 1:
Here we generate page links to the current component, retaining the
sort
parameter. We may or may not have to retain unrelated parameters (that depends on whether we're in subcase a or b).Sample for case 2:
Consider a
<Wizard>
component that has astep
querystring parameter. It's not a@page
component as you embed it in a routable page.It doesn't own the whole URL, so need to just trigger navigations to "current URL except with a modified parameter".
Possible designs
1. Do nothing
We could leave developers to figure something out on their own. In subcases (a), this isn't too hard, as developers can create a utility method that takes whatever combination of params they want and formats a
string
:However it's harder if you need to retain unrelated parameters. You end up needing to parse the existing querystring.
If you're on Blazor Server, you can use WebUtilities:
Or you can use
QueryBuilder
fromHttpExtensions
:If you're on Blazor WebAssembly, it's more messy because you don't have a reference to WebUtilities, and you can't even reference the current version as it's not a package. You'll probably end up referencing the 2.2.0 version, which is on NuGet.
Pros:
Cons:
Overall, I think this might be OK as a first step. We could wait to see what level of customer demand emerges for making this more built-in.
2. Dictionary-based API
We could expose a dictionary-based API, just like
QueryHelpers
:Pros:
Dictionary
alreadyCons:
QueryHelpers
and this new thing are both available, when they basically do the same thingDictionary<string, object>
and do the formatting for them, but that's even more allocateyGenerally I think this is fine for "navigating during an event handler" (scenario 2), but would be pretty bad for "emitting a lot of links during rendering" (scenario 1).
3. Fluent builder API
Thanks to various improvements in modern .NET, we could create an efficient fluent API for constructing or modifying querystrings that doesn't allocate at all until it writes out the final
string
instance:Or, since it's so compact, use it directly during rendering logic:
Pros:
NavigationManager
to let you update the existing URL.string
(see notes below). Also no work to construct dictionaries.Cons:
.Remove(name)
)Overall, this seems like the gold-standard solution but it's not yet 100% clear that it warrants the implementation effort.
Implementation notes: I made a prototype of this. It avoids allocations by using a
fixed
buffer on a mutablestruct
andString.Create
, plus the allocation-freeQueryStringEnumerable
added recently. It's fairly involved, but customers don't need to see or care how it works internally. In the prototype, you can add/overwrite/remove up to 3 values before it has to allocate an expandable buffer (which is also transparent to callers).The text was updated successfully, but these errors were encountered: