Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization of the $moment() object is very inconsistent #6

Open
jhorbulyk opened this issue Feb 28, 2018 · 1 comment
Open

Serialization of the $moment() object is very inconsistent #6

jhorbulyk opened this issue Feb 28, 2018 · 1 comment

Comments

@jhorbulyk
Copy link
Contributor

jhorbulyk commented Feb 28, 2018

Consider the following scenarios:

var jsonataMoment = require('@elastic.io/jsonata-moment');
jsonataMoment('$moment(1519834345000)').evaluate().toString();    // 'Wed Feb 28 2018 17:12:25 GMT+0100'
jsonataMoment('$moment(1519834345000)').evaluate() + '';    // '1519834345000'
jsonataMoment('"The time was: " & $moment(1519834345000)').evaluate()   // 'The time was: "2018-02-28T16:12:25.000Z"'

It would be nice if this was more consistent so that people don't get confused.

The reason why this is happening is as follows:

  1. $moment() evaulates to an object, not a string. If a JSONata expression writer wants a string, they should use $moment().format().
  2. For the first case, the moment() object is stringified with moment's .toString() function
  3. For the second case, the moment() object is stringified with moment's .valueOf() operator
  4. For the third case, the following is happening:
    1. JSONata's & operator is applied. As per the documentation:

      If either or both of the operands are not strings, then they are first cast to string using the rules of the $string function.

    2. JSONata's $string() operator is applied. As per the documentation:

      Casts the arg parameter to a string using the following casting rules
      [...]
      All other values are converted to a JSON string using the JSON.stringify function

    3. JS's JSON.stringify() is applied. As per the documentation:

      If an object being stringified has a property named toJSON whose value is a function, then the toJSON() method customizes JSON stringification behavior: instead of the object being serialized, the value returned by the toJSON() method when called will be serialized.

    4. Moment.js' .toJSON() is invoked. This results in JSON.stringify(someISOString) which results in an ISO string wrapped in double quotes being concatinated with the & operator.
@jhorbulyk jhorbulyk changed the title Serialization of the $moment() object is very inconsitent Serialization of the $moment() object is very inconsistent Feb 28, 2018
@jhorbulyk
Copy link
Contributor Author

The short term solution is to have @khanzadyan document the fact that integrators writing JSONata must stringify all moment() objects and to tell QA that stringifying moment() directly is a test case with undefined behavior.

Otherwise, there are the following options:

  1. Disable direct stringification of moment() in jsonata-moment by having moment's .toJSON() throw an exception and by throwing an exception if the end result of an evaluation is a moment() object.
  2. Make the JSONata stringification of moment() objects well defined. In order to do that, both of the following need to be completed:
    1. JSONata $string() function should be modified so that when called on an object, it calls a toJsonataString() function if it exists.
    2. We modify jsonataMoment so that if the evaluated result is a moment object, we stringify that object as the last step of the evaluation process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant