Try removing possibly-unused overloads in WebEventCallbackFactoryEventArgsExtensions

[SteveSandersonMS]

Trying to figure out if these are still ever used for anything.

[SteveSandersonMS]

@pranavkm @javiercn @ajaybhargavb

As part of work on #17552, I'm looking into the end-to-end flow for events and how all the types work. I might be wrong, but it looks like the extension methods in WebEventCallbackFactoryEventArgsExtensions don't get used anywhere in the code generated by the Razor compiler. It seems like the generic overload of the method is what we'd always use.

I did a bit of archaeology and found a commit 2 years ago by @rynowak that first introduced them, with the commit message containing:

The fix for this will be two-phase by first creating a set of APIs that
can be targeted by the compiler that has the desired behaviour and then
updating the compiler to target this new infrastructure.

It's quite possible that these specialized extension methods were only intended to be a temporary step, and have long since ceased to be needed. But the other possiblity is something more subtle, like maybe they change how the overload resolution works in order to produce better compiler error messages in cases where people provide incorrect types, or maybe they are only intended to influence intellisense in the design-time code or something. It's hard to say since this build (without those extension methods) and its tests passes.

If anyone here feels they know whether these extension methods or needed, or has a suggestion about how to determine that, please say so :) Since they are public, it's debatable whether we should obsolete them, remove them, or leave them alone, but regardless I'd still like to know what they are for and whether custom event args will somehow behave less well due to not having equivalent extension methods.

[ghost]

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:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[SteveSandersonMS]

Well, everything is still passing after removing these. It does not appear that they are used either in any of the code we'd generate, nor any code that we'd expect customers to write (or think they have any reason to want to write).

As such I do propose to remove them. Marking as api-ready-for-review because it technically impacts public API, even though most likely doesn't impact anyone's real-world code.

[halter73]

Why not obsolete these first?

[rynowak]

Well, everything is still passing after removing these. It does not appear that they are used either in any of the code we'd generate, nor any code that we'd expect customers to write (or think they have any reason to want to write)

It's also worth considering whether these would have been targeted by previous combination of C# compiler + Blazor compiler. For instance I suspect these overloads would have been chosed by a pre-C# 7.3 compiler: https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-7#improved-overload-candidates - if that code is out in the wild in compiled form it will not be binary compatible if these are removed.

I think we both recall a time when the Blazor templates had to specify LangVersion and I think it was due to this change. My memory tells me that this was a workaround to try and give users without access to C# 7.3 the best possible experience. However my memory often has its own agenda these days.

[pranavkm]

The plan is to mark this type as Obsolete and remove it in a future release of ASP.NET Core.

[SteveSandersonMS]

As per comment above obsoleting instead, this PR is now retired in favour of one of the commits inside #29993