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

Add exported IsNullable method to nullable types #442

Closed
wants to merge 1 commit into from

Conversation

Kalbi
Copy link
Contributor

@Kalbi Kalbi commented Mar 25, 2021

I was very happy to see #430 merged and am now using it. However, in my project, I felt that having an exported IsNullable method would be useful, especially when implementing Nullable custom scalar types.

This will allow one to verify that custom Nullable implementations correctly implement the NullUnmarshaller interface.

It also allows us to verify the type of the input field for things like gRPC field masks.

E.g., previously, if one had a traditional nullable field, we would not be able to generate gRPC field masks accurately for partial updates because if a field was nil we would not add the field mask. Now, users can check if a field IsNullable, and if it is, check if the Set field is true or false, in order to set a field mask accurately.

Hopefully this method is safe to export; it would be helpful for our project and I hope it will be useful to others who want to take advantage of the new Nullable functionality.

@pavelnikolov
Copy link
Member

Hi @Kalbi,
all these types already have a public Nullable() {} method. This means that you can create an interface that has this method and test for yourself if the type implements it. I am not convinced that we need to expose an additional static method IsNullable. This doesn't add any functionality that is not already available. At the cost of one additional interface, you can check for yourself.

@Kalbi Kalbi deleted the export-is-nullable branch March 26, 2021 00:14
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 this pull request may close these issues.

2 participants