DateString returning unexpected format via API

[charlie-s]

Description/Steps to reproduce

1. Create a model with a DateString property:

{
  "name": "Invoice",
  "base": "PersistedModel",
  "properties": {
    "number": {
      "type": "number",
      "required": true
    },
    "date_placed": {
      "type": "DateString",
      "required": true
    }
  }
}

2. Create or retrieve an instance of that model.

For example, POST the following request body to https://loopback-date-strings.herokuapp.com/api/Invoices)

{
  "date_placed": "2018-10-08",
  "number": "1111"
}

Link to reproduction sandbox

https://loopback-date-strings.herokuapp.com

Expected result

Response body is unescaped JSON and contains the date as a string.

Additional information

Response body contains escaped JSON and contains the date within this escaped JSON, as a property of when:

{
  "number": 1111,
  "date_placed": "{\"when\":\"2018-10-08\"}",
  "id": 1
}

Perhaps the formatting of the DateString property is expected, but it is not clear to me why this data type behaves this way as I've never seen another property come over from my LoopBack API's as an escaped JSON string.

The DateString type overrides the native toJSON implementation with this:

/**
 * Returns the JSON representation of the DateString object.
 * @returns {String} A JSON string.
 */
DateString.prototype.toJSON = function() {
  return JSON.stringify({
    when: this.when,
  });
};

Does anyone know why? Pinging @kjdelisle since I believe he wrote this, and #1356 (comment) which is the only discussion I can find related to this.

[bajtos]

Hello @charlie-s, thank you for reporting this issue. To be honest, I am not sure what should be the right behavior here :(

[charlie-s]

Thanks for posting, @bajtos. This issue was quite difficult to track down, I had to read up on the entire history of DateString conversations and wrap my head around where it landed. It seemed like the solution was already implemented and everything was perfect – then suddenly I got to this last stage and had to scratch my head!

Perhaps the toJSON should simply be returning a normal object, not stringified, but that doesn't explain the existence of this when property. @kjdelisle – do you know why it's setup this way?

[bajtos]

Perhaps the toJSON should simply be returning a normal object, not stringified, but that doesn't explain the existence of this when property.

Maybe toJSON should be returning just the string - that would seem most logical to me.

{
  "number": 1111,
  "date_placed": "2018-10-08",
  "id": 1
}

My biggest concern about changing the implementation of toJSON is the impact on existing applications - we don't want to break them.

On the other hand, I don't see how to current behavior (JSON output) can be useful to anybody, especially since this value cannot be fed back to DateString - i.e. a rountrip "get value from DB" and "save the same value back" will fail (if my understanding is correct).

I am proposing to introduce a feature flag - a static property on DateString that will control behavior of toJSON: by default, the old behavior is preserved for backwards compatibility. When the flag is set to true, new algorithm is used and toJSON returns just the date string.

Alternatively, instead of implementing a feature flag in juggler, maybe you can override DateString.prototype.toJSON in your application? That way no changes in juggler are needed.

// server/boot/fix-datastring-json.js
const DateString = require('loopback-datasource-juggler/lib/date-string.js');

module.exports = function(app) {
  DateString.prototype.toJSON = function() {
    return this.toString();
  };
};

I think this may be the best solution for LB2/LB3 applications. For LoopBack 4, I'd like us to address date-only strings more holistically, see loopbackio/loopback-next#1966.

What do you think?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.