models.d.ts Field<T = GenericFieldValue> is missing nullable on value field

[marcelgruber]

Description

I noticed an issue in the @sitecore-jss/sitecore-jss node package whereby in models.d.ts the generic field definition is as follows:

export interface Field<T = GenericFieldValue> {
    value: T;
    editable?: string;
}

value is assumed to never be null. However, we know that the value can be null.

This is problematic because TypeScript will not flag value as a potential null ref:

However, if I manually modify models.d.ts as follows:

export interface Field<T = GenericFieldValue> {
    value?: T;
    editable?: string;
}

I see the warning that I would expect:

bar is a Droptree field.

The source of the issue seems to be here: https://github.com/Sitecore/jss/blob/ece5908c20b961f26135cf1d6dca56d07763e51a/packages/sitecore-jss/src/layout/models.ts#L133

I suspect there was a reason that this decision was made, but it is not clear why.

Possible Fix

Change:

export interface Field<T = GenericFieldValue> {
    value: T;
    editable?: string;
}

To:

export interface Field<T = GenericFieldValue> {
    value?: T;
    editable?: string;
}

Your Environment

Using the latest version.

[pzi]

Having it as nullable type would cause a lot of work for projects that have strictNullCheck turned on but for the right reasons as it would stop issues/explosions when consuming Sitecore data. At this stage it might also be a good idea to introduce some util to check if value is null:

export const isField = <T = GenericFieldValue>(field: object): field is Field<T> =>
  typeof field === 'object' && field !== null && 'value' in field && Boolean(field.value);

Or the util itself could be introduced for more confidence when consuming fields.

[illiakovalenko]

@marcelgruber Thank you for your feedback We have an example of TreeList in our styleguide addon here Essentially it's required for you to provide the exact type (like we did), in case you have nullable values you can use Field<string | null | undefined> generic value will be optional. Please, let us know whether it fits for you. Like @pzi said, in case it's optional, it will require to have additional checks, which will provide more issues

[stale[bot]]

This has been automatically marked as stale because it has not had recent activity. It will be closed if there is no further activity within 30 days. You may add comments or the 'keep' label to prevent it from closing. Thank you for your contributions.