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

ITypedElement.InstanceType: nullable or not? #2750

Open
ewoutkramer opened this issue Mar 27, 2024 · 0 comments
Open

ITypedElement.InstanceType: nullable or not? #2750

ewoutkramer opened this issue Mar 27, 2024 · 0 comments
Labels
breaking change This issue/commit causes a breaking change, and requires a major version upgrade enhancement to be triaged

Comments

@ewoutkramer
Copy link
Member

For most usecases, ITypedElement.InstanceType is not nullable. However, when parsing incorrect data (e.g. json with an unknown property), the InstanceType cannot be determined and is null. Commonly, this will be detected after parsing, when errors are reported and the ITypedElement instance is subsequently abandoned. However, code dealing with parsing source nodes and serializers etc working with ITypedElement will have behaviour to correct for such null InstanceTypes and are expecting InstanceType to be null.

So, it is both nullable (in the context of parsing) and non-nullable (in almost every other context).

Having InstanceType nullable means that even for common usage, users have to deal with a Nullable InstanceType, which is a nuisance.

We can do three things:

  • Accept that it is nullable, and let everyone deal with it.
  • Make it non-nullable, but silently return null, so those who care can still deal with it.
  • Return a sentinel InstanceType (like Base, or a really clearly non-existing type name), so the specialized consumers can still handle it, while the other consumers do not have to bother with it.

In all cases, we have determined that this would be a breaking change.

@ewoutkramer ewoutkramer added enhancement breaking change This issue/commit causes a breaking change, and requires a major version upgrade labels Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue/commit causes a breaking change, and requires a major version upgrade enhancement to be triaged
Projects
Status: Todo
Development

No branches or pull requests

2 participants