[8.0.100-preview.5.23276.4]Unable to cast object of type when passing data(custom type) to a component in Blazor Apps after building

[Junjun-zhao]

Application Name: SPOT Client
OS: Windows10 21h2
CPU: x64
.NET Build: 8.0.100-preview.5.23276.4
App, App Source or detailed repro information checking at: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1829070/

Verify Scenarios:

1. Windows10 21h2 x64 + dotnet-sdk-8.0.100-preview.5.23276.4: Fail
2. Windows10 21h2 x64 + dotnet-sdk-8.0.100-preview.3.23178.7: Pass
3. Windows10 21h2 x64 + 7.0.302: Pass
4. Windows10 21h2 x64 + 6.0.408 Pass

App repro steps on repro machine:
1.Build and Publish the app against .NET 8 Preview 5 build.
2.Execute the app.
3.Launch url: https://localhost:5005.
4.Click “Login” button.
5.Type 9191 for pin.
6.Click “Continue” button.
7.Click “Cancel” button.
8.Click “Not Now” button.
9.Type “Brown” in the search box.
10.Select “Brown, Alex”.
11.Click “Pickup” button.
12.Click “Card” button.
13.Click “Add another card” button.

Expected Result: Add card window opens.
Actual Result: Window doesn't open.

Findings

For this app, we were able to build and publish successfully, but there is an error at runtime in one of our cases. This issue only happens after building the app with.NET 8 Preview 5 SDK , it does not repro if it was built/published with the .NET 8 Preview 3, .NET 7, or .NET 6 SDK build.

in a page, app passes data to a component (DateModalInput) like this:

<DateModalInput PlaceHolder="@(S.ExpirationPlaceholderCaption)" />

Component receives data like this:

[Parameter] public string Placeholder { get; set; }

S.ExpirationPlaceholderCaption data type is not string, but it works when we build and run it with .NET6, .NET7 and .NET 8 Preview 3 SDK.

For .NET8 Preview 5 SDK, we need to change Parameter data type, so it doesn't need to cast.

Error :

System.InvalidOperationException: Unable to set property 'PlaceHolder' on object of type 'XYZ.Platform.Client.UI.Components.DateModalInput'. The error was: Unable to cast object of type 'XYZ.Platform.StringProperty' to type 'System.String'.
 ---> System.InvalidCastException: Unable to cast object of type 'XYZ.Platform.StringProperty' to type 'System.String'.
   at Microsoft.AspNetCore.Components.Reflection.PropertySetter.CallPropertySetter[TTarget,TValue](Action`2 setter, Object target, Object value)
   at Microsoft.AspNetCore.Components.Reflection.ComponentProperties.<SetProperties>g__SetProperty|2_0(Object target, PropertySetter writer, String parameterName, Object value)
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.Components.Reflection.ComponentProperties.<SetProperties>g__SetProperty|2_0(Object target, PropertySetter writer, String parameterName, Object value)
   at Microsoft.AspNetCore.Components.Reflection.ComponentProperties.SetProperties(ParameterView& parameters, Object target)
   at Microsoft.AspNetCore.Components.ComponentBase.SetParametersAsync(ParameterView parameters)
   at Microsoft.AspNetCore.Components.Rendering.ComponentState.SetDirectParameters(ParameterView parameters)
   at Microsoft.AspNetCore.Components.

@dotnet-actwx-bot @dotnet/compat

[Junjun-zhao]

@marcpopMSFT
This bug was found in .NET 8 Preview 5 validation, could you help confirm whether it is blocker for .NET 8 Preview 5? Please help move to the right area if this is not correct. Thank you.

[mkArtakMSFT]

Looks like there may have been a compiler change? @jaredpar, @chsienki are you guys aware of this change? Was it intentional?

My personal opinion is that the error is valid, and the user code should be fixed. The fact, however, that this used to work is the concern.

[danroth27]

Can we get more details on the type of ExpirationPlaceholderCaption? How was that property getting cast to a string previously? Can we see the type?

[chsienki]

@mkArtakMSFT @danroth27 Not sure of anything that intentionally changed here. I'll take a look.

[chsienki]

@mkArtakMSFT @danroth27 @jaredpar @jjonescz

This is caused by the following change: dotnet/razor#8286, which comes from #46562

Previously, having an implicit cast to e.g. string on an object component parameter could cause the wrong AddAttribute overload to be chosen for component params, which later caused type errors when the thing was looked up and found to be a string rather than the expected object type.

We fixed this by calling the new AddComponentParameter(...) method, or adding an (object) cast if the runtime doesn't have the new method, forcing the correct overload to be called.

In this case here we're hitting the (object) version of the code gen: the object in question was castable to string so we previously chose the string overload of AddAttribute. Because the underlying object on the parameter is actually a string, it doesn't have the issue at runtime, because the types match, and the application functioned correctly.

In the new code gen we cast the underlying object directly to object so it never undergoes the explicit string conversion. When we then try and assign that to a string field at runtime it fails.

I think it's fair to say that the previously observed behavior worked unintentionally, and we've 'fixed' it with that PR, but I'm not sure how many other cases of this are in the wild and if we want to try and think about how we can maintain the existing behavior. Note that even though the code path will be different on a runtime with AddComponentParameter the outcome will be the same.

For reference:

Previous code:

        __builder.AddAttribute(108, "DateFormat", RuntimeHelpers.TypeCheck<string>("{0:MM/dd}"));
        __builder.AddAttribute(109, "PlaceHolder", (string) (MetaStoreProperty<string>) this.S.ExpirationPlaceholderCaption);
        __builder.AddAttribute(110, "Value", (object) RuntimeHelpers.TypeCheck<DateTime?>(this.expDate));
        __builder.AddAttribute<DateTime?>(111, "ValueChanged", RuntimeHelpers.TypeCheck<EventCallback<DateTime?>>(EventCallback.Factory.Create<DateTime?>((object) this, RuntimeHelpers.CreateInferredEventCallback<DateTime?>((object) this, (Action<DateTime?>) (__value => this.expDate = __value), this.expDate))));
        __builder.AddAttribute(112, "ValueExpression", (object) RuntimeHelpers.TypeCheck<Expression<Func<DateTime?>>>((Expression<Func<DateTime?>>) (() => this.expDate)));

New code:

        __builder.AddAttribute(108, "DateFormat", (object) RuntimeHelpers.TypeCheck<string>("{0:MM/dd}"));
        __builder.AddAttribute(109, "PlaceHolder", (object) this.S.ExpirationPlaceholderCaption);
        __builder.AddAttribute(110, "Value", (object) RuntimeHelpers.TypeCheck<DateTime?>(this.expDate));
        __builder.AddAttribute(111, "ValueChanged", (object) RuntimeHelpers.TypeCheck<EventCallback<DateTime?>>(EventCallback.Factory.Create<DateTime?>((object) this, RuntimeHelpers.CreateInferredEventCallback<DateTime?>((object) this, (Action<DateTime?>) (__value => this.expDate = __value), this.expDate))));
        __builder.AddAttribute(112, "ValueExpression", (object) RuntimeHelpers.TypeCheck<Expression<Func<DateTime?>>>((Expression<Func<DateTime?>>) (() => this.expDate)));

[SteveSandersonMS]

I think it's fair to say that the previously observed behavior worked unintentionally, and we've 'fixed' it with that PR, but I'm not sure how many other cases of this are in the wild and if we want to try and think about how we can maintain the existing behavior.

Thanks for investigating and writing up the info! So it seems we fixed the case where implicit casts caused problems by taking effect too eagerly (by making them not take effect), but with the unfortunate side-effect that in this case where the app wants the implicit cast to take effect, it no longer does.

I'm unsure if there is a practical way to have implicit casts take effect only in the cases where a developer wants them to. The first thing that comes to mind is if we used RuntimeHelpers.TypeCheck in all cases, e.g.:

// The declared type of the Placeholder parameter is 'string'
__builder.AddComponentParameter(109, "PlaceHolder", RuntimeHelpers.TypeCheck<string>(this.S.ExpirationPlaceholderCaption));

... so that this.S.ExpirationPlaceholderCaption would be accepted as a valid parameter value here if it's a string or something with an implicit cast to a string, but not in any other case. I haven't dug deeply into it this morning so it's possible this is insufficient or misses the point in some way. TBH I don't recall why we use RuntimeHelpers.TypeCheck<T> in some cases but not all of them.

Equivalently we could change AddComponentParameter to AddComponentParameter<T> so it reduces to the following:

// The declared type of the Placeholder parameter is 'string'
__builder.AddComponentParameter<string>(109, "PlaceHolder", this.S.ExpirationPlaceholderCaption);

My guess is that unless we get other reports from preview releases that people are hitting problems here, it's quite possibly a sufficiently minor breaking change that we might accept living with it for .NET 8. The workaround (explicitly insert a cast at the call site) is not wildly unreasonable.

[danroth27]

Unless we get other reports from preview releases that people are hitting problems here, it's quite possibly a sufficiently minor breaking change that we might accept living with it for .NET 8.

I think we want to avoid breaking changes if at all possible. As our usage grows even a small breaking change can impact a significant number of users. We want to avoid aggravating users' fear that updating to new .NET versions might break their app. With a platform back-compat is really one of our most important features.

[SteveSandersonMS]

Totally agreed, however it's not an absolute and does have to be balanced against the risks of causing other breaks and/or having to revert the original bugfix.

To understand this balance a bit more, hopefully @chsienki will be able to comment on the practicality of fixing it (for example, whether my AddComponentParameter<T> suggestion might cover it).