Aspnetcore: Microsoft.AspNetCore.Components.BindConverter TryConvertTo<T> does not support Int16

[kdawg1406]

Describe the bug

Microsoft.AspNetCore.Components.BindConverter TryConvertTo does not support Int16

Source code :

https://github.com/aspnet/AspNetCore/blob/c74e9efd0e6ea5b4d6737d23b92042772a50a249/src/Components/Components/src/BindConverter.cs

Search the source, Int16 or short do not appear in the file.

To Reproduce

1. In Blazor Client-Side app
2. Use the TryConvertTo method to attempt to convert an Int16 (short) or Int16? (short?)

Actual behavior

Exception is thrown

Expected behavior

Expect the out result value will be an Int16

[javiercn]

@kdawg1406 thanks for contacting us.

I don't believe we currently support Int16. Is there a reason why you can't simply use Int32?

[kdawg1406]

@javiercn yes, I know that Int16 was omitted which why I wrote this bug up. Please fix this, real-world applications use Int16 and so do databases.

[javiercn]

@kdawg1406 We will take this into consideration and get back to you.

We'll have to make sure there are no drawbacks regarding things like overflows, etc that can lead to a poor experience/pit of failure.

[kdawg1406]

@javiercn It's a generic method. This provides any guard you need. Also, you have Int32, Int64, etc. without issues. This is a very simple fix. Thanks!

[julienGrd]

i have the same kind of problem with byte. It's true sometimes we don't have the hand on all the code. For me i consume existing wcf services where the previous developers put some byte. I can handle it by put some shortcut properties which cast the byte in int32 on the getter and cast the int32 in byte in the setter, but its really ugly. It can be nice to have a support for this.

[kdawg1406]

@julienGrd @javiercn I was also going to point out byte as well but didn't want to push my luck. The bottom line, this converter should support all primitive numeric data types.

[mkArtakMSFT]

Thanks everyone. We will accept a PR for this, if you would like to speed things up 👍

[mrpmorris]

Thanks everyone. We will accept a PR for this, if you would like to speed things up 👍

@mkArtakMSFT I might have a go at this, but I have a few questions

1: Would it also be okay if I changed the method names so they are DotNet themed rather than C# themed?

For example TryConvertToLong should be TryConvertToInt64
https://github.com/aspnet/AspNetCore/blob/c74e9efd0e6ea5b4d6737d23b92042772a50a249/src/Components/Components/src/BindConverter.cs#L594

2: Same for these types https://github.com/aspnet/AspNetCore/blob/602cb1dea59751a1297d674cddf7f0729c23e6ca/src/Components/Web/src/Forms/InputNumber.cs#L12

3: Is it okay to implement all numeric types?

4: If so, should I just remove this line? https://github.com/aspnet/AspNetCore/blob/602cb1dea59751a1297d674cddf7f0729c23e6ca/src/Components/Web/src/Forms/InputNumber.cs#L12

5: Did you know the E2E tests in the master branch are failing? e.g. https://github.com/aspnet/AspNetCore/blob/602cb1dea59751a1297d674cddf7f0729c23e6ca/src/Components/test/E2ETest/Tests/BindTest.cs#L677

6: Should I base off master, or another branch?

[kdawg1406]

@mrpmorris if you submit a PR, can you also fix Decimal? I think that would be a breaking change if you changed to .NET Names. I prefer .NET Name, but this would be a breaking change to the public API.

[mrpmorris]

@mrpmorris if you submit a PR, can you also fix Decimal? I think that would be a breaking change if you changed to .NET Names. I prefer .NET Name, but this would be a breaking change to the public API.

Fix decimal? What's wrong with it?

[kdawg1406]

@mrpmorris If you create a new Blazor app and try to Bind to a Decimal property, it fails. That failure is tied to this class.

[javiercn]

A few notes.

• Names should not be changed, as it would be a breaking change.
• This might require changes in the compiler too, to generate the right code in these cases.

[mrpmorris]

@mrpmorris if you submit a PR, can you also fix Decimal?

I just tried decimal and it seems to work fine. Do you have a ticket number for the problem?

[kdawg1406]

@mrpmorris I'm on the 3.0 release, not working. Thank you for verifying it's fixed now.

[mrpmorris]

A few notes.

Names should not be changed, as it would be a breaking change.
This might require changes in the compiler too, to generate the right code in these cases.

@javiercn

Okay. These should be changed in future though as they are against the standards used in the rest of the framework.

What about questions 3,4,5,6?

[mrpmorris]

@javiercn @kdawg1406 I've submitted a PR for this.

@javiercn Please let me know if you would like anything changed.

[rynowak]

Thanks @captainsafia !