Change how generation of optional types works

[aaronrosenthal]

Currently the library generates interfaces using TypeScript's shorthand for optional parameters, but I believe this is incorrect or at the very least should be configurable.

For example, these two are equivalent:

interface MyType {
  myProp?: number
}

interface MyType {
  myProp: number | undefined
}

Generally serializers are going to be used to render JSON in a controller, and since JSON doesn't serialize undefined values (only null values), I think the above behavior could be improved. Without any changes, the only workaround I've been able to find would be doing something like:

attributes my_prop: { type: "number | null" }

or simply leave it out of the serializer with:

attributes :my_prop, if: -> { object.my_prop.present? }

neither of which are ideal as they lead to a lot of extra boilerplate, especially for types inferred from the schema.

What are thoughts on this? I suggest one (or both) of two solutions:

Solution 1

Configuration setting to change the global behavior so that all optional parameters are generated in a syntax like the following:

interface MyType {
  myProp: number | null
}

or perhaps:

interface MyType {
  myProp?: number | null
}

so undefined values are also considered. This could look something like the following, where we can specify whether to use just undefined (this would be the default, and could continue to generate shorthand syntax), just null, or both:

config.optional_parameter_types = [:undefined, :null]

Solution 2

An option in oj_serializer to by default exclude all null values in serialized responses (so they actually would be undefined in TypeScript):

class BaseSerializer < Oj::Serializer
  omit_nulls
end

[ElMassimo]

Hi Aaron!

Thanks for the thoughtful discussion.

Adding support for optional: :null and optional: :null_or_undefined to the library would provide more accurate types for the serialized data.

For example, SQL columns without defaults would either have a value or end up as null (but never undefined), so it would make sense to treat them as optional: :null.

On the other hand, conditional attributes would be undefined when the condition is not met, and conditional attributes for a nullable column could be automatically inferred to be optional: :null_or_undefined.

[aaronrosenthal]

That's a great point about conditional attributes. Thinking about this, I believe for conditional attributes there's no need at all to generate types allowing null, with one exception: I think flat_one still needs to handle the case where the condition is met but the flattened attributes may still be null (just like normal attributes). This could be the only case where the properties need to support both null and undefined.

Also I agree with you that inferred types from SQL don't need to support undefined in the TypeScript interface.

Instead of specifying the optional type on each attribute, do you think this could be a configuration to globally apply null types (so the library maintains backward compatibility)? If so I'd be happy to submit a PR.