-
Notifications
You must be signed in to change notification settings - Fork 893
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
FR: Add valueOf() to Timestamp instances #2632
Comments
Hi @davidwkeith Thanks for you suggestion. We are a little hesitate to implement We might consider exposing its internal |
I assume that since the library is written in Typescript that it would be OK to use {
valueOf(): BigInt {
const secondsStr = this.seconds.toString();
const nanosecondsStr = this.nanoseconds.toString().padStart(10, '0')
return BigInt([secondsStr, nanosecondsStr].join(''))
}
} |
Thanks, I think this will work. We will close this issue when the change is merged. |
Actually, Safari does not support BigInt, we are looking into if we could still represent Timestamp with a |
This enables comparison of Timestamp objects using the arithmetic comparison operators, such as < and >. #2632
This enables comparison of Timestamp objects using the arithmetic comparison operators, such as < and >. #2632
I've opened a ticket for this same issue in the |
This enables Timestamp objects to be compared for relative ordering using the arithmetic comparison operators (i.e. <, <=, >, and >=). Fixes firebase/firebase-js-sdk#2632
For posterity, here is the "packet" that I sent to the API Review Council for adding the valueOf() method to the timestamp class. Background InformationThe When JavaScript compares two object references using these operators, it first converts them to primitives by invoking the objects' Here is an example adapted from the bug report from the external developer that shows the "unconditionally-false" behavior that is observed when comparing a = new Timestamp(1, 1)
b = new Timestamp(2, 2)
console.log(a < b) // false, which is incorrect
console.log(b < a) // false Proposed API ChangeThe proposed API change is to override the The behavior of the sample code above would become the following: a = new Timestamp(1, 1)
b = new Timestamp(2, 2)
console.log(a < b) // true, which is correct
console.log(b < a) // false This proposal has been peer reviewed and is awaiting review from API council before merging into master: #2662. Alternatives ConsideredAlternative 1: Do NothingMaking no changes to the Alternative 2: Return
|
This enables comparison of Timestamp objects using the arithmetic comparison operators, such as < and >. Fixes: #2632
[REQUIRED] Describe your environment
[REQUIRED] Describe the problem
Unable to compare Timestamp instances with native comparators. Javascript calls
valueOf()
on the object and uses the results of that to do native comparisons.Steps to reproduce:
Attempt to compare Timestamps directly.
Relevant Code:
Workaround
Of course you can simply call
toMillis()
and compare those, but having it auto handled would be better.The text was updated successfully, but these errors were encountered: