dojo / widget-core

:rocket: Dojo 2 - widget authoring system.
http://dojo.io
Other
34 stars 39 forks source link

Provide default text node treatment for common types within vdom.ts #868

Open economatics opened 6 years ago

economatics commented 6 years ago

When using expressions as children of TSX nodes, the evaluated result is used to be handled as a widget within vdom.ts. It fails on that with errors when result is a common JS type.

To fix it please provide something similar to vdom.ts at line 473:

else if (typeof child === 'string' || typeof child === 'number' || typeof child === 'boolean' || child instanceof Date) {

Nevertheless there may be the same problem when evaluated result isinstanceof array. Looks like the whole array may be treated as widget then. Am I right?

agubler commented 6 years ago

@economatics do you mind providing code examples of your scenario so we can determine if there’s anything we can sensibly support as a default in vdom?

economatics commented 6 years ago

@agubler thanx for attending :)

A use case may be to provide large, hierarchical properties to a widget that implements its render function with tsx. Those props may consist of dynamical data that has to be integrated into rendering. To be more precise, I need it for a table component that has to be able to handle different types of data. Domain data may be provided in different shapes that than are treated as table rows. A cell of a row than has to render a single attribute of a domain object. It does not know anything of the data's shape. it just includes them as it's children:

class Cell extends TemplateWidget {
    protected render(): DNode {
      const { classes, style } = this.properties;
      return (
        <div
          classes={classes}
          style={style}
          role="gridcell"
        >
          {this.children}
        </div>
      );
    }
  }

Currently, when children consists of a number, or boolean vdom.ts crashes as described above.

agubler commented 6 years ago

The interface for children (DNode[]) does not support number or boolean so it actually shouldn't be passed as children to any widget. If you were using the w() function instead of TSX then you would see errors:

// Argument of type 'boolean[]' is not assignable to parameter of type 'DNode<DefaultWidgetBaseInterface>[] | undefined'.
//  Type 'boolean[]' is not assignable to type 'DNode<DefaultWidgetBaseInterface>[]'.
//    Type 'boolean' is not assignable to type 'DNode<DefaultWidgetBaseInterface>'.
w(MyClass, properties, [ true ]);

// Argument of type 'number[]' is not assignable to parameter of type 'DNode<DefaultWidgetBaseInterface>[] | undefined'.
//  Type 'number[]' is not assignable to type 'DNode<DefaultWidgetBaseInterface>[]'.
//    Type 'number' is not assignable to type 'DNode<DefaultWidgetBaseInterface>'.
w(MyClass, properties, [ 1 ]);

Unfortunately I don't think this type guarding is possible with TSX, we have an issue open for this https://github.com/dojo/widget-core/issues/675.

Domain data may be provided in different shapes that than are treated as table rows. A cell of a row than has to render a single attribute of a domain object. It does not know anything of the data's shape. it just includes them as it's children:

It is always sensible to guard against the data types passed into your widgets, this is usually done by defining the type of the properties on widget, but if it's something that is truly any then in the cell widget I would recommend parsing the single attribute using a template string ${value}.

Having said this we I've marked this as an enhancement that we'll triage post release to see whether we can solve this generically within vdom without causing too many significant overheads (complexity, breakages, performance etc).