dateTime API is removed at v2.0

[shinxi]

Looks dateTime API just removed...and developers need to manually assign a moment object to px-calendar-picker. Is there a way we still support string APIs? In some use cases like polymer and react integration, we can't assign a moment object to a polymer Element under react environment.

[benoitjchevalier]

A bit of background: we decided to get rid of dateTime because keeping the moment object and the string in sync proved to be a constant source of bugs and we needed to have a single source of truth. Moment being the underlying object the datetime components rely on it stood out as the best choice.

What are the limitations preventing you from assigning a moment object to a web component in react?

[shinxi]

Here's our story, let's say we have a React CalendarPicker component.

class CalendarPicker extends React.Component {
  render() {
    const dateTime = this.props.dateTime.toISOString(); 
    return <px-calendar-picker date-time={dateTime} />
  }
}
// Usage
<CalendarPicker dateTime={new Date("2017-12-20T02:06:06.388Z")}>

Previously we use dateTime and it works perfectly with polymer v1 + px-calendar-picker v1. But after upgrade to polymer v2(long story, polymer v2 is proved to work best with react since it's based on shadowDOM v1), dateTime just can't work, and the initial date will always be the current date (a constant source of bugs it is). So we tried px-calendar-picker v2, but dateTime API is removed and only left momentObj, the thing is we can't assign an object value to px-calendar-picker, pls remember data-binding from polymer will not work for CalendarPicker and px-calendar-picker, CalendarPicker is just JSX rather than Polymer Element, if a property value is object type, we have to serialize it to string firstly like polymer guide mentioned, maybe you can add a deserializer for momentObj and we just pass momentObj with momentObj.toISOString(), or make dateTime readOnly so that it won't reduce a bunch of bugs?

class CalendarPicker extends React.Component {
  render() {
    const momentObj = momentObj; 
    return <px-calendar-picker moment-obj={momentObj} />
  }
}
// Usage
<CalendarPicker momentObj={Px.moment().subtract(7, 'days')}>

In short, we need a string API at px-calendar-picker v2 and other date components v2 version like px-datetime-picker, px-rangepicker(range is removed) etc. Thanks in advance.

[mdwragg]

@davidrleonard isn’t this where that really awkward looking ‘refs’ syntax comes into play?

@shinxi - I seem to recall doing this via binding in JSX just doesn’t work. If only React could handle instances and setting properties gracefully!

[shinxi]

@shinxi - I seem to recall doing this via binding in JSX just doesn’t work. If only React could handle instances and setting properties gracefully!

Just corrected some typo 😆

<px-calendar-picker dateTime={dateTime} />

should be

<px-calendar-picker date-time={dateTime} />

The first one is just a very simple polymer to react integration sample, actually, we are doing more, like auto converting React camelCase property to Polymer dashCase property, auto binding polymer events etc. IMHO, It can't blame React on the second momentObj case since they are two different data system, and even polymer itself, it just supports string value at the template if without Polymer data-binding.

[davidrleonard]

Background on this problem

Unfortunately the issue goes a bit deeper than this API change. The core problem is that React JSX rendering system stringifies all unknown attributes. If we embrace that limitation, we'd never be able to expose a web component property that can't be sent through JSON.stringify/JSON.parse. That would mean we couldn't support properties that take complex objects with non-serializable data inside them (e.g. objects with a function as a value, used extensively elsewhere in the design system) or properties that take references to complex type instances (e.g. reference to HTMLElement for event binding, also used extensively).

This is a known limitation of React and JSX, with a proposal for a fix that has been open for a while here: https://github.com/facebook/react/issues/7249. Preact have embraced the web component specification and added support for setting unknown attributes by property in templates if the developer wants. All other major JavaScript frameworks have also embraced the web component specification and similar support setting unknown attributes by property.

We would very much support this change coming to React and the JSX templating system. If you have time to weigh in on that issue thread, or to reach out to Facebook and advocate for that change, we’d be happy to help and provide you with any details you need.

Our recommendation to solve this

We recommend you wrap each web component that takes non-primitive attribute values in a React component. You already showed some examples of doing that — that’s awesome. The next step is to find the DOM ref of the web component and send updates down to the component when your React wrapper’s props change.

Here’s a very reduced case that shows how you could extend the CalendarPicker component shared above to support this:

class CalendarPicker extends React.Component {
  componentDidMount() {
    // When first mounted, convert dateTime prop to moment object and pass it
    // down to the web component using its ref
    if (this.props.dateTime) {
      this.calendarPickerNode.momentObj = this.dateTimeToMoment(this.props.dateTime);
    }
  }

  // When props change, check for a diff. If there's a diff, update the web
  // component using its ref
  componentWillReceiveProps(nextProps) {
    if (nextProps.dateTime && this.props.dateTime !== nextProps.dateTime) {
      this.calendarPickerNode.momentObj = this.dateTimeToMoment(this.props.dateTime);
    }
  }

  // Convert dateTime ISO string to moment object
  dateTimeToMoment(dateTime) {
    return Px.moment(dateTime.toISOString());
  }

  render() {
    return <px-calendar-picker ref={n => {this.calendarPickerNode = n}} />
  }
}

This uses the React refs API to find the web component in the dom, converts the dateTime prop to a moment object, and sets the momentObj property on the web component to update it.

I added a bit of sugar into the component that will allow you to continue to use the dateTime API wherever your React app does so today so this upgrade shouldn't be too painful. You'd need to make the dateTimeToMoment method handle the format you expect for props.dateTime, in this case I just copied your example of passing a native JavaScript Date instance.

[shinxi]

Thanks @davidrleonard for the detailed background, looking forward https://github.com/facebook/react/issues/11347 can be implemented one day. One question

(e.g. objects with a function as a value, used extensively elsewhere in the design system

Do you mean the value of a predix component attribute is a function, is that exposed to users or used inside predix component? Can you give more samples, since so far our integrated predix components, most properties are primitive or array or object.

Also thanks for the recommendation, and yes, we've already used ref for registering polymer events, we even know how to solve this dateTime issue at React side for px-calender-picker v1 + polymer v2, but we are not quite fond of this sort of hack-code at React side, CalenderPicker is just one sample, we have many other similar date components like DateTimePicker(px-datetime-picker), DateTimeInput(px-datetime-field), RangePicker(px-rangepicker), RangeInput(px-datetime-range-filed) etc. More importantly, we might face more issues adding the fix at React. E.g, a more realistic sample when working with events.

class CalendarPicker extends React.Component {
  componentDidMount() {
    this.calendarPickerNode.addEventListener('moment-obj-changed', this.props.onMomentObjChanged);
    // When first mounted, convert dateTime prop to moment object and pass it
    // down to the web component using its ref
    if (this.props.dateTime) {
      this.calendarPickerNode.momentObj = this.dateTimeToMoment(this.props.dateTime);
    }
  }

  // When props change, check for a diff. If there's a diff, update the web
  // component using its ref
  componentWillReceiveProps(nextProps) {
    if (nextProps.dateTime && this.props.dateTime !== nextProps.dateTime) {
      // triggers moment-obj-changed everytime.
      this.calendarPickerNode.momentObj = this.dateTimeToMoment(this.props.dateTime);
    }
  }

  // Convert dateTime ISO string to moment object
  dateTimeToMoment(dateTime) {
    return Px.moment(dateTime.toISOString());
  }

  render() {
    return (
      <px-calendar-picker
        ref={n => {
          this.calendarPickerNode = n;
        }}
      />
    );
  }
}

class Demo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      dateTime: new Date('2017-10-24T02:18:57.388Z'),
    };
  }

  onMomentObjChanged = event => {
    this.setState({
      dateTime: new Date(event.detail),
    });
  };

  render() {
    return (
      <CalendarPicker
        ref={n => {
          this.calendarPickerNode = n;
        }}
        dateTime={this.state.dateTime}
        onMomentObjChanged={this.onMomentObjChanged}
      />
    );
  }
}

CalendarPicker#componentWillReceiveProps => Demo#onMomentObjChanged => CalendarPicker#componentWillReceiveProps An execution loop happens, and page will get stuck. And in order to fix this, we have to add shouldComponentUpdate check at Demo component like...

shouldComponentUpdate = (nextProps, nextState) => this.state.dateTime !== nextState.dateTime;

If implement this feature (dateTime, range) originally at polymer implementation rather than React workaround, that'll be the most convenient way for us.

[davidrleonard]

A few quick things....

1. I understand why you're hitting the execution loop above. It's true that the date-time property exposed before made it easier to avoid this because string equality in JavaScript is by value, not by reference. Unfortunately the maintenance overhead for us was too high. If you'd like to break this execution loop, introduce the following equality checks:

  componentWillReceiveProps(nextProps) {
    // The API we've mocked out always has the prop `dateTime` as a native JS Date instance
    // so we can use the `getTime()` method to get primitive numbers that are friendly
    // to a strict equality check. Now prop updates that aren't actually different won't trigger
    // a new moment-object-changed event
    if (nextProps.dateTime && this.props.dateTime.getTime() !== nextProps.dateTime.getTime()) {
      ...
    }
  }

  onMomentObjChanged = event => {
    // evt.detail.value is a reference to the new moment instance,
    // we can call `toDate()` to get a native JS Date instance then
    // `getTime()` to get a primitive number to do an equality check
    if (evt.detail.value.toDate().getTime() !== this.state.dateTime.getTime()) {
      ...
    }
  };

2. To answer your question about non-serializable attribute/property APIs in Predix UI components, there are quite a few. For example, px-app-nav#selected property relies on referential equality between JavaScript objects and can't be used with serialized/deserialized values. The px-context-browser#selected, px-context-browser#active, px-tree#selected, and px-breadcrumbs#selected all rely on referential equality as well. The px-vis charts shared dynamicMenuConfig property relies on the developer passing an object with values that are functions, which cannot be passed through JSON.stringify/JSON.parse. The forthcoming px-data-table major upgrade relies on passing objects with function values for many advanced configuration cases. These are just a few examples.

I understand your frustration, but we're going to go ahead with this API to ensure continued stability and speed to delivery of new features for the datetime components. Thanks for weighing in on the API. Please feel free to continue giving us feedback on our APIs. And please make your voice heard with the Facebook React team — we'd love for React to join the other modern frameworks in implementing patterns that make the developer experience of using web components 💯.

[shinxi]

Ok... Two more question.

For example, px-app-nav#selected property relies on referential equality between JavaScript objects and can't be used with serialized/deserialized values. The px-context-browser#selected, px-context-browser#active, px-tree#selected, and px-breadcrumbs#selected all rely on referential equality as well.

It looks we can also use #selectedRoute this array property instead of the referential #selected property, right?

The forthcoming px-data-table major upgrade relies on passing objects with function values for many advanced configuration cases.

Does that mean we'll have to do similar workarounds at React for next-generation px-data-table?

[mdwragg]

OK, so I'm going to close this thread down. We have reworked the API of the date time components based on consultation from many "in production" customers of these components. The changes that we have shipped provide great advantage for those customers, huge gains in maintainability for us and bring up the reliability of these components to where it needs to be, all the while allowing the flexibility and interop of using Moment to initialise, configure and interact with the components.

I suggest you work with our TPM @Aileen-H if the above path of wrapping this component (which you are doing anyway I believe?) doesn't work for you.

Thanks, Martin Wragg, Director - Design Technology, Predix Design System Engineering Lead.