Node-red-contrib-moment - breaking update - looking for input

#1

Hi all. I'm looking for input about a potential change to my node-red-contrib-moment node.

Currently, there is a slight issue when you supply data that isn't a date/time. According to the build-in warning, you should get an empty string as output. However, the documentation says that the current date/time will be used as input instead.

Whilst the documentation version is likely more useful to people, I'm concerned that a few people might be relying on the output being empty text. If you are using this node, can you please let me know which you think would be best?

0 Likes

#2

@TotallyInformation, has this issue been closed by the release of v3.0.0? (I'm not seeing it yet on GitHub.) If so, how have you resolved it?

BTW, many thanks (again) for this enormously useful node.

0 Likes

#3

I personally would prefer an empty string. Most of the time when I've used your node (awesome btw) it's been to either translate or migrate time series data. A blank timestamp would be much easier to filter out as an error case than a valid but inaccurate timestamp.

0 Likes

#4

+1 for me also - i would prefer invalid input gives a blank/NULL output

Craig

0 Likes

#5

No, v3 has fixed a bug with input that is null (that case was producing an odd date in the past for some reason) and has updated Moment.JS and other libraries to current versions. But I decided to leave the overall processing as-is for now, pending the outcome of this discussion.

It seems like the consensus is to leave things as-is so that's what I will do. Thanks for the feedback.

Invalid inputs - any input that exists but cannot be converted to a date/time - will result in an empty string as output along with a debug warning message. If the input doesn't exist at all (e.g. you pass a non-existent msg/global/flow property) results in the current date/time being used as input.

Although I forgot to update the docs, you can also use the following convenience strings as input: "today", "yesterday", "tomorrow".

It is a pleasure to give something back to the community. I still have plans to add a couple more nodes to the library for other types of processing such as more detailed date/time calculations and duration processing. I also need to update the main node to allow more inputs from the msg. Sadly I always seem to be out of time these days. But PR's always welcome if you want to have a crack at something :smile:

0 Likes

#6

Agreed -- in the case of invalid input, does the node issue a warning that could be caught?

0 Likes

#7

Currently, it doesn't. It issues a node.warn message but this isn't picked up by the catch or status nodes.

However, since an invalid input results in an empty string on the output, you can catch it that way using a switch node.

0 Likes

#8

That makes sense. @JayDickson describes an valuable use case that should be preserved. I often use moment to attach a time stamp to a message when it reaches a particular point in a flow. There are plenty of ways to do that without breaking existing code, which is why I haven't followed up on "fixing" the issue of invalid string inputs.

I think I understand what was happening here. moment.js was accepting null as a valid date, and interpreting it as zero. The result was 1970-01-01T00:00:00.000Z (or its local equivalent), which is time zero of the UNIX Epoch. Perhaps reasonable but not useful.

0 Likes

#9

Thanks for the feedback. I guessed that null was being treated as a zero. Didn't seem right to me so it is no more! :slight_smile:

Obviously, if anyone has any further suggestions, please feel free to raise here or on GitHub.

0 Likes