Add IgnoreQueryString parameter to NavLink

[igotinfected]

Add IgnoreQueryString parameter to NavLink

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

I've repurposed a stale PR (#32168) that gives users more control on NavLink matches when query strings are present. Do note that while writing the missing E2E tests, I noticed that the original implementation would not cover the case where a query string followed a trailing slash (i.e. /abc/?value=123). This is now covered by trimming a trailing slash if it is present when the user asks for the query string to be ignored.

Fixes #31312

[dnfadmin]

All CLA requirements met.

[mkArtakMSFT]

Thanks for your PR, @igotinfected.
@javiercn can you please review this? Thanks!

[mfluehr]

I think this PR would be very useful. I hope it goes through.

[TanayParikh]

Hmm, I wonder if this behavior could potentially be treated as a subset of #18163?

[igotinfected]

@TanayParikh Reading through that issue made me realise this IgnoreQueryString implementation would also ignore the Fragment part of the URI, if specified after a query string, and in other words, this implementation does not ignore fragments if they are the only extension to the base path.

I'd be interested in taking a look at the ShouldMatch approach if that's the direction we want to go in, just let me know ☺️

[javiercn]

I don't think we need a new flag for this. This is just a bug/oversight in existing code

[SteveSandersonMS]

I don't think we need a new flag for this

Are you referring to the whole IgnoreQueryString flag, or to some other hypothetical new flag related to @TanayParikh's comment about the hash part of the URL?

If you mean IgnoreQueryString itself, I agree it would be better if we were ignoring the querystring by default. However that would be a breaking change we would need to weigh up carefully.

[javiercn]

I don't think matching on the query string would have offered a good behavior before. Query strings are not ordered, so the chances of matching on the query string would have been incredibly narrow.

I think we can just make the change and make an announcement about it if we deeply care about it.

[SteveSandersonMS]

I agree about the default, however there are clear cases where someone might have wanted it to match on querystrings and might in fact be depending on that behavior now. For example, if you have:

@page "/products"

@code {
    [Parameter]
    [SupplyParameterFromQuery(Name = "category_id")]
    public int CategoryId { get; set; }
}

... then you might also have navigation links like:

<NavLink href="products?category_id=1">Dogs</NavLink>
<NavLink href="products?category_id=2">Cats</NavLink>
<NavLink href="products?category_id=3">Fish</NavLink>

... and expect those links to light up to show the current page.

It's not the most common pattern, but it's a valid and believable one so we should be prepared for making a clear announcement and treating it as a breaking change if we're going to change this.

[javiercn]

Yep, I'm fine if we do that.

The alternative is to properly support query string parameters, but that is much more work.

I want to avoid the situation where we have a behavior that "half" works and I think in this case it is simpler to not support it at all vs to add full support for it.