tomasherceg commented 4 years ago

This is not a single issue - its purpose is to track several steps that need to be done in order to allow the implementation of several planned features.

[x] Change the generator of validation maps to return complete type information
[x] Update client-side validation code to use the new typeMetadata structure
[x] Implement coercion and validation logic
[x] Update serializer/deserializer to use typeMetadata instead of $options
[ ] Implement support for TimeSpan
[ ] Implement support for the createInstance method

Thanks to the strongly-typed nature of DotVVM, we've found out that it would be useful to have a complete type information about the structure of the viewmodel on the client. In the first place, it will allow the users to work safely with DotVVM viewmodels on the client. Also, it will help us to bring new features like immutable-state viewmodels which will hopefully lead to a better client-side performance.

Supported types in the viewmodels

The viewmodel of the page ($root) must always be an object. DotVVM doesn't assume that the root viewmodel would be an array or a primitive type - if this is intended, such type can be wrapped as the only member of an object type.

The viewmodel can contain properties of the following supported types:

primitive types (see the table further)
object types (that meet the same criteria as the viewmodel)
arrays of supported primitive types or supported object types

Supported primitive types

The following table lists the primitive types that are supported in DotVVM.

Server type	Client type	Notes
bool	boolean
byte	number
sbyte	number
short	number
ushort	number
int	number
uint	number
long	number	Possible precision loss
ulong	number	Possible precision loss
float	number
double	number
decimal	number	Possible precision loss
string	string
char	string
Guid	string
Enum	string
DateTime	Date	Until DotVVM 3.0, date values were stored as strings in the viewmodel
TimeSpan	Date	New in DotVVM 3.0

Nullability

Until DotVVM 3.0, all properties in the viewmodel were naturally nullable, and even if declared as non-nullable on the server, the null value could legally appear there on the client (in case of a validation error where the user's value couldn't be parsed).

From DotVVM 3.0, we should be more strict and have the information about nullability or non-nullability available on the client.

Type metadata for viewmodels - current state

DotVVM has some information about the viewmodel types, but it is far from being complete.

validationRules map

Currently, DotVVM emits "validation rules" for every type that is used in the viewmodel. This metadata is not part of the viewmodel - they are sent alongside with the viewmodel on the first request for the page.

  "validationRules": {
    "x/VCL7UIamx+ULk7atmiE2Ealrg=": {
      "ActualTime": [
        {
          "ruleName": "enforceClientFormat",
          "errorMessage": "The field ActualTime is invalid.",
          "parameters": [
            true,
            true,
            true
          ]
        }
      ],
      "ChristmasEventEndDate": [
        {
          "ruleName": "enforceClientFormat",
          "errorMessage": "The field ChristmasEventEndDate is invalid.",
          "parameters": [
            true,
            true,
            true
          ]
        }
      ]
    }
  }

The key in the validationRules map is a unique identifier of the type on the server. Each type holds its properties (right now only those with validation attributes).

Each object in the viewmodel has an extra $type property which contains this identifier, so the validation can then load the rules that need to be checked.

$options entries in the viewmodel

Another piece of incomplete type information we have, is the Property$options entries in the viewmodel objects. These entries contain information about bind direction (dotNotPost) as well as information about types (isDate for date values or type for numeric types).

     ...
     "AlertText": null,
      "AlertText$options": {
        "doNotPost": true
      },
      "AlertType": "Success",
      "AlertType$options": {
        "doNotPost": true
      },
      ...

This is quite unfortunate as these values are often repeated (especially in large collections), and the only reason for them is that the client-side serializer is not type-aware.

Proposed format of type metadata

We'll need to extend these two structures to hold complete type information for each type used in the viewmodel.

Also, when the viewmodel gets changed on postback, a new type can appear there - we'll need to send the information about the new types in the viewmodel.

"typeMetadata": {
    "x/VCL7UIamx+ULk7atmiE2Ealrg=": {
        "ActualTime": {
            "type": "DateTime",
            "nullable": false,
            "direction": "ServerToClient"
        },
        "Age": {
            "type": "int",
            "nullable": true,
            "direction": "Both",
            "validationRules": [
                {
                    "ruleName": "enforceClientFormat",
                    "errorMessage": "The field Age is invalid.",
                    "parameters": [
                        true,
                        true,
                        true
                    ]
                }
            ]
        },
        "Items": {
            "type": "array",
            "elementType": "chdEgsaGsASG+/135=",
            "nullable": false,
            "direction": "Both"
        }
    },
    "chdEgsaGsASG+/135=": {
        "Id": {
            "type": "int"
        }
    }
}

The type property can be either:

"Server type" from the primitive types table above
array in case of a collection - then there is a required elementType property that specifies the type of the collection items
a unique identifier of another type specified in the typeMetadata object

The nullable property is false by default.

The direction can contain values from the Direction enum on the server - default is Both

Validation & Coercion

Instead of allowing anyone to write anything in the viewmodel, we'll have to create an API that is able to validate data that are written in the viewmodel. This API would be called everywhere where the viewmodel is updated - e.g. patch method. We probably don't need to update it in the deserializer as we'll trust that server will produce correct responses.

We also need to be able to define coercion functions for all primitive types. Some components may try to write numeric values as strings and so on - it would be great if DotVVM could fix some of these issues on the client-side than rely on try/catch block in deserializer on the server to handle such situations.

Transport conversions

Some values may need to be transferred in a different form than they are represented in the viewmodel (e.g. date values should be transferred as strings in order to prevent messing up with timezones when the server and client have different date offset).

Therefore, we'll also need function to convert the value to transport representation, and from transport representation.

Proposed API

I'd like to introduce something like dotvvm.typeMap which would offer the following functions:

getTypeMetadata will return the entire typeMetadata structure with all known types
validate(value, TYPE_NAME) will validate the specified object according to the type metadata definition
createInstance(TYPE_NAME) will create a new instance of the specified type - it can be used later for translation of the new operator in data-binding expressions
primitiveTypes.TYPE_NAME.coerceValue(value) will try to coerce the value to the specified type, or will throw an error if it is not possible
primitiveTypes.TYPE_NAME.toTransportValue(value) will return transport representation (e.g. string representation of a date value)
primitiveTypes.TYPE_NAME.fromTransportValue(value) will return "natural" value from its transport representation (e.g. string representation of a date value)

exyi commented 4 years ago

Just a few comments on some details of the specification:

Arrays

Your proposal does not handle nested arrays well, we could not have an array with "elementType": "array". I think we could just use the value "type": [elementType] instead of separate properties. This also simplifies the format, IMHO.

Integer types

We currently send the type information about exact integer type for range checking. The format used is int32 or uint32 where 32 is the number of available bits.

Nullability

We currently send information on nullability of numbers, as it is needed for essential validation. It is denoted by a ? at the end of the type name and I'd stick to that notation even for other types. The same as with the arrays - it keeps the metadata more concise, type information is in one field and thus easier to work with.

On the other hand, I'm not sure we want to enforce (non-)nullability of reference types, it may cause a lot of problems to some users. The benefits are not super clear.

Currently, null values are permitted in the model, it only fails validation, so postbacks can not be initiated. Do check that only before postbacks or on every update? The second option may be quite problematic for some components that store null in viewmodel where they get an invalid value.

Bind direction

I think that sending all information from the enum is just a complication for the client-side code - I guess we only need to know whether to send this value to server or not (which is only 3 states - yes, no, when in datacontext path)

Coecion and transport

The fromTransportValue seems quite redundant - we can just the coercion. The coercion is also mostly going to convert strings into the proper type.

The API

validate is going to perform only type validation or the validation that we call validation? :D In the first case, I'd move it to dotvvm.validation, in the second case I'd not call it validate for clarity. I guess we can use single method for conversion and for validation.

The primitiveTypes.TYPE_NAME... complicates implementation of int validation a bit, as we parse the size of the int, we do not have all int types hardcoded. I'd simply pass the typeName as a parameter. Also, what is the reason to only support primitive types, it seems natural to call the toTransport function on entire object.

tomasherceg commented 4 years ago

Thanks for the feedback.

Arrays

You are right, elementType doesn't support nested arrays - we'll use just type with the full specification of the array. However, I'd prefer to use the C# syntax so I suggest int[] instead of [int].

Integer types

We'll check the range on the client-side based on the type name - we'll probably need to have some mapping table there instead of parsing the number of bits from the type name. Alternatively, we could use not C# aliases but .NET type names - Int32 and so on.

Nullability

Agree, we can use the ? and I am also afraid of actually supporting non-nullability in viewmodels - the benefits are not significant (DotVVM treats every . more like ?. so the users don't experience NRE even if something is null).

Maybe we could emit Required validator for all non-nullable reference type properties used in the viewmodel, but still - it is a breaking change and I am not sure about the impact.

Bind direction

I've run into some glitch where the trio of "post", "do not post" and "post if in postback path" wasn't enough when using ServerToClientFirstRequest in combination with server-side viewmodel caches, but I don't remember the details - maybe it wasn't the concern on the client-side.

Let's try it and we'll see if it breaks or not.

Coercion

I think that it would be better to distinguish between these two types of conversions (even if they'll have the same implementation). For example, coercion from string to date can allow more formats (basically anything that the user can provide), but fromTransportValue should accept only one exact format DotVVM uses.

Or is it because we are using the same deserialize method for deserialization JSON responses from the server as well as assigning in static commands? In that case, we can unify it.

The API

You are right - validate is not a good name. Maybe we could just have coerceValue and toTransportValue that would work for all types (recursively). Coerce would throw an error if it cannot be performed.

Or do we need to recognize if there was actually some coercion or if the value conformed exactly with the specified type? Something like "strict mode"?

exyi commented 3 years ago

However, I'd prefer to use the C# syntax so I suggest int[] instead of [int].

The point is, that ["int"] can be written in JSON, so we would not have to parse it.

Agree, we can use the ? and I am also afraid of actually supporting non-nullability in viewmodels - the benefits are not significant (DotVVM treats every . more like ?. so the users don't experience NRE even if something is null).

The problem is with value types in view model - we can not send null to the server as it will just fail during validation. For this reason, we check on client-side that the fields are not null. Checking reference types would be a very annoying breaking change, we could add that behind a flag or so.

I think that it would be better to distinguish between these two types of conversions (even if they'll have the same implementation). For example, coercion from string to date can allow more formats (basically anything that the user can provide), but fromTransportValue should accept only one exact format DotVVM uses.

Or is it because we are using the same deserialize method for deserialization JSON responses from the server as well as assigning in static commands? In that case, we can unify it.

Kinda, it just seems to make it more complicated to have two ways of updating viewmodel. And we'd have to use the less strict version in static commands, since we don't really distinguish between server invocation and user code invocation.

tomasherceg commented 3 years ago

I have started implementing the type metadata in the viewmodel-types branch. It's not working right now, but the generation of type metadata JSON works and is covered by a few tests.

Type names

The point is, that ["int"] can be written in JSON, so we would not have to parse it. Nice idea - we can take advantage of the TypeScript type system instead of parsing. Right now, the types look like this:

primitive types: "Boolean", "Int32"... I've decided to use .NET type names (not C# aliases)

enums: { "type": "enum", "values": { "Zero": 0, "One": 1, "Two": 2 } } - I think that the numeric values will also be useful

nullables: { "type": "nullable", "inner": <innerType> } - because of enums which are not string, I couldn't just append ?

arrays or collections: [ <innerType> ]

complex types: "sha1-of-full-type-name"

Also, the client can send a list of types for which it already has metada (the knownTypeMetadata: [ "hash1", "hash2", ... ] entry in postback or static command request payload) - the serializer will only return new types.

Bind direction

I've also changed the bind direction information to this:

client to server direction: "post": undefined | "no" | "pathOnly" (undefined means "always")
server to client direction: "update": undefined | "no" | "firstRequest"(undefined means "always")

acizmarik commented 3 years ago

Implement support for TimeSpan

We decided that for now we are not going to work on supporting TimeSpans. We might decide to add this support in later versions of DotVVM.

Reason: In order to support this, we would basically need to provide our own JS implementation for TimeSpanParse.cs and TimeSpanFormat.cs as it is not supported by the globalize library. At this point it seems to be too much work, not many people requested this feature and partial workarounds are possible using DateTime.

riganti / dotvvm

Client-side ViewModel Specification #882

Supported types in the viewmodels

Supported primitive types

Nullability

Type metadata for viewmodels - current state

validationRules map

$options entries in the viewmodel

Proposed format of type metadata

Validation & Coercion

Transport conversions

Proposed API

Arrays

Integer types

Nullability

Bind direction

Coecion and transport

The API

Arrays

Integer types

Nullability

Bind direction

Coercion

The API

Type names

Bind direction