fix: support null in DeepPartial when useNullAsOptional

[LuckyEden]

Added null to the Builtin union when useNullAsOptional=true, so that DeepPartial<T> correctly preserves null values.

[stephenh]

Lmk if you're fine adding the test; happy to merge this either way. Thanks!

[stephenh]

I didn't think of this approach (adding null to Builtin), but it's passing all the tests, so sgtm! :-D

We can merge this as-is, but fwiw it might be a good idea to create a use-null-as-optional.test.ts file, next to the use-null-as-optional.ts file and do something like:

const u = User.createPartial({
  // ideally this was a compile error w/o your change, but now works
  profile: null,
});
expect(u.profile).someAssertion();

Just to make sure we don't accidentally regress this change in the future.

Happy to merge this as-is, but just will be some caveats that I could see someone thinking "null shouldn't be in Builtin" down the road, and not realize they're breaking your use case.

[LuckyEden]

Yes, you are right. I will add testing to make sure it is covered. I am also thinking of making it parametric to cover a few variations more easily. Does that make sense? Or will just test be enough? Maybe I can test the ORM compatibility more?

[LuckyEden]

I have a question, if the value is null in a generate operation with the useNullAsOptional option, shouldn't it be considered null?

The default is undefined;

message.profile = (object.profile !== undefined && object.profile !== null)
      ? ProfileInfo.fromPartial(object.profile)
      : undefined;

Maybe it can be done with other options, I need to review them again. Sorry if it sounds ridiculous. :D

[stephenh]

Hi @LuckyEden , sorry for the late reply here; yeah, I think you're right...

Historically ts-proto leaned towards my own bias of "always use undefined" and so would convert null and undefined to "just undefined" for simplicity.

Primarily b/c on the wire, protobuf doesn't have a way to different "what is null" vs. "what is undefined" so I didn't think it made sense for the programmer to have to write code that was like "if undefined do this, if null do that" when really only one of those ifs would ever run.

However with useNullAsOptional, I'd have to refresh my memory on what that opt is for 😅 but it seems reasonable for that : undefined to be : null...