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

Beyond 3DP seconds causes incorrect format output #96

Open
TotallyInformation opened this issue May 25, 2019 · 5 comments
Open

Beyond 3DP seconds causes incorrect format output #96

TotallyInformation opened this issue May 25, 2019 · 5 comments

Comments

@TotallyInformation
Copy link

TotallyInformation commented May 25, 2019

The following code:

var moment = require('moment')
var parseFormat = require('moment-parseformat')

var dp6 = "2019-05-25T15:30:42.526058+00:00"
var dp5 = "2019-05-25T15:30:42.52605+00:00"

var long6dp = moment(dp6)
var long5dp = moment(dp5)

var fmt6dp = parseFormat(dp6,{preferredOrder: {'/': 'DMY', '.': 'DMY', '-': 'YMD'}})
var fmt5dp = parseFormat(dp5,{preferredOrder: {'/': 'DMY', '.': 'DMY', '-': 'YMD'}})

var fmt6dpMoment = moment(dp6, fmt6dp)
var fmt5dpMoment = moment(dp5, fmt5dp)

console.log( 'long6dp', long6dp.format() )
console.log( 'long5dp', long5dp.format() )
console.log( 'fmt6dp parseFormat', fmt6dp )
console.log( 'fmt5dp parseFormat', fmt5dp )
console.log( 'fmt6dpMoment', fmt6dpMoment.format() )
console.log( 'fmt5dpMoment', fmt5dpMoment.format() )

Produces the following output:

long6dp 2019-05-25T16:30:42+01:00
long5dp 2019-05-25T16:30:42+01:00
fmt6dp parseFormat YYYY-MM-DDTHH:mm:ss.SSSDDDZ
fmt5dp parseFormat YYYY-MM-DDTHH:mm:ss.SSSDDZ
fmt6dpMoment 2019-02-27T15:30:42+00:00
fmt5dpMoment 2019-05-05T16:30:42+01:00

As you can see, anything beyond 3DP on the seconds value results in "D" specifications on the end of the fractional seconds which is clearly incorrect and produces uncertain output from moment.

Reported to me in the following issue:

TotallyInformation/node-red-contrib-moment#24

@gr2m
Copy link
Owner

gr2m commented May 26, 2019

It's not really within the scope of the project, moment-parseFormat is meant to parse date/time strings entered by humans.

Something like 2019-05-25T15:30:42.52605+00:00 is probably better parsed by using JavaScript’s native Date object

new Date('2019-05-25T15:30:42.52605+00:00')

@TotallyInformation
Copy link
Author

A point. However, it shouldn't be very hard to fix - my regex skills are not the sharpest but it looks as though it would be possible to change the regex to include any number of fractional seconds. Though I think that >6 would fail in MomentJS.

Also you do already support the same format to 3dp.

The worst thing is that there is no warning so people entering longer strings may suddenly get an incorrect date/time output and may never realise.

@gr2m
Copy link
Owner

gr2m commented May 28, 2019

I can’t tell from looking at it how complex it would be. Each feature we’d add would need to be supported until the end of time, and I certainly won’t need it myself.

I suggest you or someone else sends a pull request to see how much added complexity would be required to add support for it, then we can make a decision

@TotallyInformation
Copy link
Author

TotallyInformation commented Mar 7, 2020

This is still occurring and becoming more of an issue as more systems support 64bit data and more systems support micro- and nano-second data.

I have a partial work-around in node-red-contrib-uibuilder but it doesn't catch every case:

    // If ISO string w/ seconds >3dp, then process else exit
    if ( inp.match(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{4,}/) ) {
        inp = inp.replace(/^(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3})\d*(.*)/, '$1$2')
    }

That is missing some data that has a +nn:nn timezone on the end. Not sure my regular expression fu is good enough to fully fix.

At the very least, it would be much better to at least return a warning because this issue can result in an incorrect date being returned.

@QbaF
Copy link

QbaF commented Mar 7, 2020

I do agree @TotallyInformation. Node moment does have problems with some data formats (as last updated/last changed) going out from some most fundamental nodes as e.g. events: state

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

3 participants