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

Timestamps not converted to Date anymore #238

Closed
kflip opened this issue Aug 27, 2015 · 5 comments
Closed

Timestamps not converted to Date anymore #238

kflip opened this issue Aug 27, 2015 · 5 comments
Assignees
Labels

Comments

@kflip
Copy link

kflip commented Aug 27, 2015

Timestamps passed as parameter to remote methods worked in older versions:
e.g.:
from=1440631417
or in milliseconds
from=1440631417000

It fails to instantiate a new Date() due a missing conversion from String to Number.
I've made a workaround in Shared-Method.js, but you might want to solve this somewhere else (HttpContext) or differently.

thanks

...
function convertValueToTargetType
...
case 'date': {

    // could be a timestamp as string
    if (!isNaN(value) && typeof value == 'string') {
        value = Number(value).valueOf();
    } 

    return new Date(value);
}
kflip added a commit to PocketScientists/strong-remoting that referenced this issue Oct 12, 2015
Signed-off-by: Philipp Kinschel <p.kinschel@nousguide.com>
@richardpringle richardpringle added this to the #Epic: Coercion Cleanup milestone May 4, 2016
@richardpringle
Copy link
Contributor

@bajtos, can you confirm if this is expected behavior or not?

If we don't expect a number to coerce, then I think there should be a warning in the docs.

@bajtos
Copy link
Member

bajtos commented May 9, 2016

Interesting. So if my understanding is correct, when we have { accepts: { arg: 'd', type: 'Date' } },
it used to be possible to send ?from=1440631417, where the value was treated as timestamp. @kflip could you please confirm? Do you happen to know the last version where this was still working?

@bajtos
Copy link
Member

bajtos commented May 27, 2016

This is actually tricky. Both new Date('0') and new Date(0) are valid ways how to create a new Date instance, however each returns a different value. It makes sense to me to always convert timestamp-like values to numbers before creating a Date value.

I have a question though. How to handle numeric values in scientific notation (e.g. ?ts=1.234e%2B30) and values that cannot be accurately represented as integer numbers (e.g. ?ts=2343546576878989879789)? I am proposing to reject such values and return HTTP 400 Bad Request response to the client.

/cc @STRML

@richardpringle
Copy link
Contributor

@bajtos can this be closed now since #349 was landed?

@bajtos
Copy link
Member

bajtos commented Sep 19, 2016

Yes, I am closing this issue as fixed (in LoopBack 3.0).

@bajtos bajtos closed this as completed Sep 19, 2016
@bajtos bajtos removed the #wip label Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants