Skip to content

Remove custom implementations for Number methods #2752

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

Merged
merged 2 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/firestore/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type MutationBatchState = 'pending' | 'acknowledged' | 'rejected';
* primarily used by the View / EventManager code to change their behavior while
* offline (e.g. get() calls shouldn't wait for data from the server and
* snapshot events should set metadata.isFromCache=true).
*
*
* The string values should not be changed since they are persisted in
* WebStorage.
*/
Expand Down
49 changes: 4 additions & 45 deletions packages/firestore/src/util/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,11 @@
* limitations under the License.
*/

// Untyped Number alias we can use to check for ES6 methods / properties.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const NumberAsAny = Number as any;

// An Object whose keys and values are strings.
export interface StringMap {
[key: string]: string;
}

/**
* Minimum safe integer in Javascript because of floating point precision.
* Added to not rely on ES6 features.
*/
export const MIN_SAFE_INTEGER: number =
NumberAsAny.MIN_SAFE_INTEGER || -(Math.pow(2, 53) - 1);

/**
* Maximum safe integer in Javascript because of floating point precision.
* Added to not rely on ES6 features.
*/
export const MAX_SAFE_INTEGER: number =
NumberAsAny.MAX_SAFE_INTEGER || Math.pow(2, 53) - 1;

/**
* Returns whether an number is an integer, uses native implementation if
* available.
* Added to not rely on ES6 features.
* @param value The value to test for being an integer
*/
export const isInteger: (value: unknown) => boolean =
NumberAsAny.isInteger ||
(value =>
typeof value === 'number' &&
isFinite(value) &&
Math.floor(value) === value);

/**
* Returns whether a variable is either undefined or null.
*/
Expand All @@ -64,19 +33,9 @@ export function isNullOrUndefined(value: unknown): boolean {
*/
export function isSafeInteger(value: unknown): boolean {
return (
isInteger(value) &&
(value as number) <= MAX_SAFE_INTEGER &&
(value as number) >= MIN_SAFE_INTEGER
typeof value === 'number' &&
Number.isInteger(value) &&
(value as number) <= Number.MAX_SAFE_INTEGER &&
(value as number) >= Number.MIN_SAFE_INTEGER
);
}

/**
* Safely checks if the number is NaN.
*/
export function safeIsNaN(value: unknown): boolean {
if (NumberAsAny.IsNaN) {
return NumberAsAny.IsNaN(value);
} else {
return typeof value === 'number' && isNaN(value);
}
}
17 changes: 8 additions & 9 deletions packages/firestore/test/unit/model/field_value.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { GeoPoint } from '../../../src/api/geo_point';
import { Timestamp } from '../../../src/api/timestamp';
import * as fieldValue from '../../../src/model/field_value';
import { primitiveComparator } from '../../../src/util/misc';
import * as typeUtils from '../../../src/util/types';
import {
blob,
dbId,
Expand All @@ -40,12 +39,12 @@ describe('FieldValue', () => {

it('can parse integers', () => {
const primitiveValues = [
typeUtils.MIN_SAFE_INTEGER,
Number.MIN_SAFE_INTEGER,
-1,
0,
1,
2,
typeUtils.MAX_SAFE_INTEGER
Number.MAX_SAFE_INTEGER
];
const values = primitiveValues.map(v => wrap(v));

Expand All @@ -62,10 +61,10 @@ describe('FieldValue', () => {

it('can parse doubles', () => {
const primitiveValues = [
typeUtils.MIN_SAFE_INTEGER - 1,
Number.MIN_SAFE_INTEGER - 1,
-1.1,
0.1,
typeUtils.MAX_SAFE_INTEGER + 1,
Number.MAX_SAFE_INTEGER + 1,
NaN,
Infinity,
-Infinity
Expand Down Expand Up @@ -438,8 +437,8 @@ describe('FieldValue', () => {
[wrap(NaN)],
[wrap(-Infinity)],
[wrap(-Number.MAX_VALUE)],
[wrap(typeUtils.MIN_SAFE_INTEGER - 1)],
[wrap(typeUtils.MIN_SAFE_INTEGER)],
[wrap(Number.MIN_SAFE_INTEGER - 1)],
[wrap(Number.MIN_SAFE_INTEGER)],
[wrap(-1.1)],
// Integers and Doubles order the same.
[new fieldValue.IntegerValue(-1), new fieldValue.DoubleValue(-1)],
Expand All @@ -453,8 +452,8 @@ describe('FieldValue', () => {
[wrap(Number.MIN_VALUE)],
[new fieldValue.IntegerValue(1), new fieldValue.DoubleValue(1)],
[wrap(1.1)],
[wrap(typeUtils.MAX_SAFE_INTEGER)],
[wrap(typeUtils.MAX_SAFE_INTEGER + 1)],
[wrap(Number.MAX_SAFE_INTEGER)],
[wrap(Number.MAX_SAFE_INTEGER + 1)],
[wrap(Infinity)],

// timestamps
Expand Down
13 changes: 6 additions & 7 deletions packages/firestore/test/unit/remote/node/serializer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ import {
import { Code, FirestoreError } from '../../../../src/util/error';
import { Indexable } from '../../../../src/util/misc';
import * as obj from '../../../../src/util/obj';
import * as types from '../../../../src/util/types';
import { addEqualityMatcher } from '../../../util/equality_matcher';
import {
bound,
byteStringFromString,
dbId,
deletedDoc,
deleteMutation,
Expand All @@ -81,8 +81,7 @@ import {
transformMutation,
version,
wrap,
wrapObject,
byteStringFromString
wrapObject
} from '../../../util/helpers';
import { ByteString } from '../../../../src/util/byte_string';

Expand Down Expand Up @@ -196,13 +195,13 @@ describe('Serializer', () => {

it('converts IntegerValue', () => {
const examples = [
types.MIN_SAFE_INTEGER,
Number.MIN_SAFE_INTEGER,
-100,
-1,
0,
1,
100,
types.MAX_SAFE_INTEGER
Number.MAX_SAFE_INTEGER
];
for (const example of examples) {
verifyFieldValueRoundTrip({
Expand Down Expand Up @@ -501,11 +500,11 @@ describe('Serializer', () => {

it('converts a long key', () => {
const actual = s.toName(
key('users/' + types.MAX_SAFE_INTEGER + '/profiles/primary')
key('users/' + Number.MAX_SAFE_INTEGER + '/profiles/primary')
);
expect(actual).to.deep.equal(
'projects/p/databases/d/documents/users/' +
types.MAX_SAFE_INTEGER.toString() +
Number.MAX_SAFE_INTEGER.toString() +
'/profiles/primary'
);
});
Expand Down