Skip to content
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

v2.1.1 introduces breaking changes #1959

Closed
ilya-scale opened this issue Sep 13, 2023 · 5 comments · Fixed by #1960
Closed

v2.1.1 introduces breaking changes #1959

ilya-scale opened this issue Sep 13, 2023 · 5 comments · Fixed by #1960

Comments

@ilya-scale
Copy link

This code does not compile anymore (if one has warnings as errors):

public class DateTimeHandler : SqlMapper.TypeHandler<DateTime>
{  
    public override DateTime Parse(object value)
    {
        return DateTime.SpecifyKind((DateTime) value, DateTimeKind.Utc);
    }
}

The reason is the new nullability.

It is probably fine, but just wanted to check if maybe doing 3.0.0 would fit better since it is breaking?

I have warnings as errors and floating version, so by just running nuget get I got the compilation error.

@mgravell
Copy link
Member

Firstly, thanks for trying hidden builds and finding this :)

I assume this is now "object?" ? Let me revisit this API and see if there are any genuine scenarios where the value is null; if there aren't, we can possibly make that "object". Otherwise: well, it is right :) In the second case - I'm not sure this warrants a "major"; it is a code warning that is readily fixed or suppressed. I imagine more people will get hit by the "OrDefault" methods returning "?", but again: it is right :)

@mgravell
Copy link
Member

@ilya-scale looks like we can tweak this 👍

mgravell added a commit that referenced this issue Sep 13, 2023
* assert that type handlers don't need object? - include test to validate (plus: NRT check was happy)

fix #1959

* release notes
@mgravell
Copy link
Member

mgravell commented Sep 13, 2023

This should be fixed in release 2.1.4; no runtime API change; build-time annotations updated

@mgravell
Copy link
Member

mgravell commented Sep 13, 2023

2.1.4 is on nuget now (it may not have been indexed yet)

@ilya-scale
Copy link
Author

Thanks for taking a look at this @mgravell , with 2.1.4 I was able to revert my changes back to non-nullable.

There were some other places that I have found similar issues, e.g.

  1. public override void SetValue(IDbDataParameter parameter, T? value) (in the same class) was just T value before
  2. public static void AddTypeHandler(Type type, ITypeHandler handler) became non nullable

I am not sure about the first case (whether it should or should not be nullable). The second ones looks correct of course as there is no need to send null in there. It is just in my case caused an error during a compilation which is kind of an indication of a major version, but I see you point that it is probably too small of a change for a Major.

Additionally, I am not sure what do you mean by "hidden builds", my configuration is:

<PackageReference Include="Dapper" Version="2.*" />

So while I see that somehow these builds are not visible on nuget website, the dotnet commands find them for sure for floating version :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants