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.

Sign up for GitHub

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

Jump to bottom

Make value converting nulls internal for 6.0 #26230

Closed
ajcvickers opened this issue Oct 1, 2021 · 1 comment · Fixed by #26232
Closed

Make value converting nulls internal for 6.0 #26230

ajcvickers opened this issue Oct 1, 2021 · 1 comment · Fixed by #26232
Assignees
@ajcvickers
Labels
area-type-mapping closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved type-enhancement
Milestone
6.0.0

Comments

@ajcvickers
Copy link
Member

In 6.0 we implemented Allow HasConversion/ValueConverters to convert nulls. However, this has proved to be very problematic in practice with many pitfalls. For example:

These are not trivial issues and for the query issues they are not easy to detect. This makes the feature somewhat a pit-of-failure.

Therefore, we are proposing to mark these APIs as internal for 6.0. This still allows people to use them, but behind an analyzer and documentation warning. (This is similar to marking them as experimental--see EntityFrameworkExperimentalAttribute) We can then make them fully public in a future release after gathering more feedback and possibly making improvements.
@ajcvickers
Copy link
Member Author

@Pilchie @JeremyLikness FYI. We can discuss as needed.

This was referenced Oct 1, 2021
Open
Open
ajcvickers added a commit that referenced this issue Oct 1, 2021
@ajcvickers
Mark internal the ValueConverter constructors that allow conversion o…
02feb86 
…f nulls.

Resolves #26230.

Also revert built-in converters that were changed to allow this in 6.0 back to their 5.0 behavior.
@ajcvickers ajcvickers mentioned this issue Oct 1, 2021
Merged
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-consider labels Oct 1, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Oct 1, 2021
@ajcvickers ajcvickers self-assigned this Oct 1, 2021
@ajcvickers ajcvickers changed the title Make value converting nullable types internal for 6.0 Make value converting nulls internal for 6.0 Oct 4, 2021
This was referenced Oct 9, 2021
Open
Open
ajcvickers added a commit to dotnet/EntityFramework.Docs that referenced this issue Oct 17, 2021
@ajcvickers
Doc updates for breaking changes and switching to daily GA build
61c8278 
* Fixes #3489: Document validation of requiredness (nullability) in the in-memory database
* Fixes #3487: Document overriding initialized navigations break
* Fixes #3180: Document enum values breaking change
* Document making value converting nulls internal for 6.0 (dotnet/efcore#26230)
* Move samples to Ga daily build
ajcvickers added a commit to dotnet/EntityFramework.Docs that referenced this issue Oct 17, 2021
@ajcvickers
Doc updates for breaking changes and switching to daily GA build
737117e 
* Fixes #3489: Document validation of requiredness (nullability) in the in-memory database
* Fixes #3487: Document overriding initialized navigations break
* Fixes #3180: Document enum values breaking change
* Document making value converting nulls internal for 6.0 (dotnet/efcore#26230)
* Move samples to Ga daily build
@ajcvickers ajcvickers mentioned this issue Oct 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ajcvickers ajcvickers

Labels
area-type-mapping closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-approved type-enhancement
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

[6.0] Mark internal the ValueConverter constructors that allow conversion of nulls dotnet/efcore
1 participant
@ajcvickers