NeVeSpl / NTypewriter

File/code generator using Scriban text templates populated with C# code metadata from Roslyn API.
https://nevespl.github.io/NTypewriter/
MIT License
126 stars 24 forks source link

Why does Type.ToTypeScriptType ignore nullable if [Required] attribute is present? #96

Closed washamdev closed 1 year ago

washamdev commented 1 year ago

We are generating form view models for our .NET 6.0 C# project, like this:

[ExportToTypeScript]
public class FulfillOrderViewModel
{
    [Display(Name = "Business")]
    public long? BusinessId { get; set; }

    [Required]
    [Display(Name = "Ad Account")]
    public long? AdAccountId { get; set; }
}

Our NT template has template code like this:

export class {{ class.BareName }} {
    {{~ for property in class.Properties | Symbols.ThatArePublic | Array.Sort("Name") ~}}
    public {{ property.Name }}: {{ Type.ToTypeScriptType property.Type }};
    {{~ end ~}}
}

This results in the following rendered TS file:

export class FulfillOrderViewModel {
    public AdAccountId: number;
    public BusinessId: number | null;
}

The reason the C# VM needs to be set up like that is because when you first land on the form webpage, the View is bound to the FulfillOrderViewModel, so we pass an empty FulfillOrderViewModel to the View for it to bind to. We don't want BusinessId, for instance, to have the default long value of 0, so we make the property nullable, but then add the [Required] attribute to the property, so that it has to be set to an actual value before the form can be submitted.

But as you can see, the Type.ToTypeScriptType method is returning number for AdAccountId, instead of number | null like it does for BusinessId. The only difference is that AdAccountId has the [Required] attribute.

Is this expected? Is my only option to create my own custom function with my own implementation of Type.ToTypeScriptType to ignore the [Required] attribute?

NeVeSpl commented 1 year ago

Yes, that is by design.

Just remember that you can invoke Type.ToTypeScriptType from your custom function, thus it should not be a big trouble to add | null in case of the presence of [Required] attribute.

washamdev commented 1 year ago

I was realizing while I was using NTypewriter this time that there are two different meanings to a property being "required":

  1. A property must always have a value (i.e. not nullable).
  2. A property is nullable but must be filled out before the class is "valid" (by using [Required] on it).

My properties are not 1, but they are 2. That made me realize that [Required] cannot be the only source of "truth" if a property is nullable or not. Seems like if a property has a nullable type, that should be the winner of if a TS type has " | null" or not, since C# is going to allow that property to be null by default. Is there something I'm missing here? Fully open to that possibility!

washamdev commented 1 year ago

@NeVeSpl Can you help me understand if I'm missing something please? I went ahead and made my own custom function, but still confused about the default behavior. Thanks!

NeVeSpl commented 1 year ago

If we consider only C# and TS languages semantics your point of view is 100% correct. But, (n)Typewriter was created in mind to be used with the asp.net framework. Since asp will enforce on runtime non-nullability for props decorated with [Required] attribute, (n)Typewriter goes further and makes sure that TS types are also non-nullable.