Skip to content

Commit 0c273f2

Browse files
schmidt-sebastianschmidt-sebastian
committed
Don't normalize Proto data during deepClone
1 parent 6ad4f58 commit 0c273f2

File tree

8 files changed

+135
-113
lines changed

8 files changed

+135
-113
lines changed

‎packages/firebase/rollup.config.js

+7-25
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ import commonjs from '@rollup/plugin-commonjs';
2121
import sourcemaps from 'rollup-plugin-sourcemaps';
2222
import typescriptPlugin from 'rollup-plugin-typescript2';
2323
import typescript from 'typescript';
24-
import { uglify } from 'rollup-plugin-uglify';
25-
import { terser } from 'rollup-plugin-terser';
2624
import json from '@rollup/plugin-json';
2725
import pkg from './package.json';
2826

@@ -111,7 +109,7 @@ const appBuilds = [
111109
format: 'umd',
112110
name: GLOBAL_NAME
113111
},
114-
plugins: [...plugins, uglify()]
112+
plugins: [...plugins]
115113
}
116114
];
117115

@@ -143,14 +141,7 @@ const componentBuilds = pkg.components
143141
{
144142
input: `${component}/index.ts`,
145143
output: createUmdOutputConfig(`firebase-${component}.js`),
146-
plugins: [
147-
...plugins,
148-
uglify({
149-
output: {
150-
ascii_only: true // escape unicode chars
151-
}
152-
})
153-
],
144+
plugins: [...plugins],
154145
external: ['@firebase/app']
155146
}
156147
];
@@ -195,14 +186,7 @@ const firestoreBuilds = [
195186
{
196187
input: `firestore/index.cdn.ts`,
197188
output: createUmdOutputConfig(`firebase-firestore.js`),
198-
plugins: [
199-
...plugins,
200-
uglify({
201-
output: {
202-
ascii_only: true // escape unicode chars
203-
}
204-
})
205-
],
189+
plugins: [...plugins],
206190
external: ['@firebase/app']
207191
}
208192
];
@@ -248,7 +232,7 @@ const firestoreMemoryBuilds = [
248232
{
249233
input: `firestore/memory/index.cdn.ts`,
250234
output: createUmdOutputConfig(`firebase-firestore.memory.js`),
251-
plugins: [...plugins, uglify()],
235+
plugins: [...plugins],
252236
external: ['@firebase/app']
253237
}
254238
];
@@ -274,7 +258,7 @@ const completeBuilds = [
274258
sourcemap: true,
275259
name: GLOBAL_NAME
276260
},
277-
plugins: [...plugins, uglify()]
261+
plugins: [...plugins]
278262
},
279263
/**
280264
* App Node.js Builds
@@ -314,8 +298,7 @@ const completeBuilds = [
314298
typescript
315299
}),
316300
json(),
317-
commonjs(),
318-
uglify()
301+
commonjs()
319302
]
320303
},
321304
/**
@@ -345,8 +328,7 @@ const completeBuilds = [
345328
json({
346329
preferConst: true
347330
}),
348-
commonjs(),
349-
terser()
331+
commonjs()
350332
]
351333
}
352334
];

‎packages/firestore/src/exp/database.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
* limitations under the License.
1616
*/
1717

18+
// eslint-disable-next-line import/no-extraneous-dependencies
1819
import {
1920
_getProvider,
2021
_removeServiceInstance,
2122
FirebaseApp,
2223
getApp
23-
// eslint-disable-next-line import/no-extraneous-dependencies
2424
} from '@firebase/app-exp';
2525
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
2626
import { Provider } from '@firebase/component';

‎packages/firestore/src/lite/database.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
* limitations under the License.
1616
*/
1717

18+
// eslint-disable-next-line import/no-extraneous-dependencies
1819
import {
1920
_getProvider,
2021
_removeServiceInstance,
2122
FirebaseApp,
2223
getApp
23-
// eslint-disable-next-line import/no-extraneous-dependencies
2424
} from '@firebase/app-exp';
2525
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
2626
import { Provider } from '@firebase/component';

‎packages/firestore/src/model/values.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,11 @@ export function isMapValue(
559559
export function deepClone(source: Value): Value {
560560
if (source.geoPointValue) {
561561
return { geoPointValue: { ...source.geoPointValue } };
562-
} else if (source.timestampValue) {
563-
return { timestampValue: { ...normalizeTimestamp(source.timestampValue) } };
562+
} else if (
563+
source.timestampValue &&
564+
typeof source.timestampValue === 'object'
565+
) {
566+
return { timestampValue: { ...source.timestampValue } };
564567
} else if (source.mapValue) {
565568
const target: Value = { mapValue: { fields: {} } };
566569
forEach(

‎packages/firestore/test/integration/api/type.test.ts

+69-72
Original file line numberDiff line numberDiff line change
@@ -33,49 +33,59 @@ apiDescribe('Firestore', (persistence: boolean) => {
3333
async function expectRoundtrip(
3434
db: firestore.FirebaseFirestore,
3535
data: {},
36-
validateSnapshots = true
37-
): Promise<void> {
36+
validateSnapshots = true,
37+
expectedData?: {}
38+
): Promise<firestore.DocumentSnapshot> {
39+
expectedData = expectedData ?? data;
40+
3841
const collection = db.collection(db.collection('a').doc().id);
3942
const doc = collection.doc();
40-
await doc.set(data);
4143

44+
await doc.set(data);
4245
let docSnapshot = await doc.get();
43-
expect(docSnapshot.data()).to.deep.equal(data);
46+
expect(docSnapshot.data()).to.deep.equal(expectedData);
47+
48+
await doc.update(data);
49+
docSnapshot = await doc.get();
50+
expect(docSnapshot.data()).to.deep.equal(expectedData);
4451

4552
// Validate that the transaction API returns the same types
4653
await db.runTransaction(async transaction => {
4754
docSnapshot = await transaction.get(doc);
48-
expect(docSnapshot.data()).to.deep.equal(data);
55+
expect(docSnapshot.data()).to.deep.equal(expectedData);
4956
});
5057

5158
if (validateSnapshots) {
5259
let querySnapshot = await collection.get();
5360
docSnapshot = querySnapshot.docs[0];
54-
expect(docSnapshot.data()).to.deep.equal(data);
61+
expect(docSnapshot.data()).to.deep.equal(expectedData);
5562

56-
const eventsAccumulator = new EventsAccumulator<firestore.QuerySnapshot>();
63+
const eventsAccumulator =
64+
new EventsAccumulator<firestore.QuerySnapshot>();
5765
const unlisten = collection.onSnapshot(eventsAccumulator.storeEvent);
5866
querySnapshot = await eventsAccumulator.awaitEvent();
5967
docSnapshot = querySnapshot.docs[0];
60-
expect(docSnapshot.data()).to.deep.equal(data);
68+
expect(docSnapshot.data()).to.deep.equal(expectedData);
6169

6270
unlisten();
6371
}
72+
73+
return docSnapshot;
6474
}
6575

6676
it('can read and write null fields', () => {
67-
return withTestDb(persistence, db => {
68-
return expectRoundtrip(db, { a: 1, b: null });
77+
return withTestDb(persistence, async db => {
78+
await expectRoundtrip(db, { a: 1, b: null });
6979
});
7080
});
7181

7282
it('can read and write number fields', () => {
73-
return withTestDb(persistence, db => {
83+
return withTestDb(persistence, async db => {
7484
// TODO(b/174486484): If we build ViewSnapshots from IndexedDb, this test
7585
// fails since we first store the backend proto in IndexedDb, which turns
7686
// -0.0 into 0.0.
7787
const validateSnapshots = !persistence;
78-
return expectRoundtrip(
88+
await expectRoundtrip(
7989
db,
8090
{ a: 1, b: NaN, c: Infinity, d: -0.0 },
8191
validateSnapshots
@@ -84,91 +94,78 @@ apiDescribe('Firestore', (persistence: boolean) => {
8494
});
8595

8696
it('can read and write array fields', () => {
87-
return withTestDb(persistence, db => {
88-
return expectRoundtrip(db, { array: [1, 'foo', { deep: true }, null] });
97+
return withTestDb(persistence, async db => {
98+
await expectRoundtrip(db, { array: [1, 'foo', { deep: true }, null] });
8999
});
90100
});
91101

92102
it('can read and write geo point fields', () => {
93-
return withTestDoc(persistence, doc => {
94-
return doc
95-
.set({
96-
geopoint1: new GeoPoint(1.23, 4.56),
97-
geopoint2: new GeoPoint(0, 0)
98-
})
99-
.then(() => {
100-
return doc.get();
101-
})
102-
.then(docSnapshot => {
103-
const latLong = docSnapshot.data()!['geopoint1'];
104-
expect(latLong instanceof GeoPoint).to.equal(true);
105-
expect(latLong.latitude).to.equal(1.23);
106-
expect(latLong.longitude).to.equal(4.56);
107-
108-
const zeroLatLong = docSnapshot.data()!['geopoint2'];
109-
expect(zeroLatLong instanceof GeoPoint).to.equal(true);
110-
expect(zeroLatLong.latitude).to.equal(0);
111-
expect(zeroLatLong.longitude).to.equal(0);
112-
});
103+
return withTestDb(persistence, async db => {
104+
const docSnapshot = await expectRoundtrip(db, {
105+
geopoint1: new GeoPoint(1.23, 4.56),
106+
geopoint2: new GeoPoint(0, 0)
107+
});
108+
109+
const latLong = docSnapshot.data()!['geopoint1'];
110+
expect(latLong instanceof GeoPoint).to.equal(true);
111+
expect(latLong.latitude).to.equal(1.23);
112+
expect(latLong.longitude).to.equal(4.56);
113+
114+
const zeroLatLong = docSnapshot.data()!['geopoint2'];
115+
expect(zeroLatLong instanceof GeoPoint).to.equal(true);
116+
expect(zeroLatLong.latitude).to.equal(0);
117+
expect(zeroLatLong.longitude).to.equal(0);
113118
});
114119
});
115120

116121
it('can read and write bytes fields', () => {
117-
return withTestDoc(persistence, doc => {
118-
return doc
119-
.set({
120-
bytes: Blob.fromUint8Array(new Uint8Array([0, 1, 255]))
121-
})
122-
.then(() => {
123-
return doc.get();
124-
})
125-
.then(docSnapshot => {
126-
const blob = docSnapshot.data()!['bytes'];
127-
// TODO(firestorexp): As part of the Compat migration, the SDK
128-
// should re-wrap the firestore-exp types into the Compat API.
129-
// Comment this change back in once this is complete (note that this
130-
// check passes in the legacy API).
131-
// expect(blob instanceof Blob).to.equal(true);
132-
expect(blob.toUint8Array()).to.deep.equal(
133-
new Uint8Array([0, 1, 255])
134-
);
135-
});
122+
return withTestDb(persistence, async db => {
123+
const docSnapshot = await expectRoundtrip(db, {
124+
bytes: Blob.fromUint8Array(new Uint8Array([0, 1, 255]))
125+
});
126+
127+
const blob = docSnapshot.data()!['bytes'];
128+
// TODO(firestorexp): As part of the Compat migration, the SDK
129+
// should re-wrap the firestore-exp types into the Compat API.
130+
// Comment this change back in once this is complete (note that this
131+
// check passes in the legacy API).
132+
// expect(blob instanceof Blob).to.equal(true);
133+
expect(blob.toUint8Array()).to.deep.equal(new Uint8Array([0, 1, 255]));
136134
});
137135
});
138136

139137
it('can read and write date fields', () => {
140-
return withTestDb(persistence, db => {
141-
const dateValue = new Date('2017-04-10T09:10:11.123Z');
142-
// Dates are returned as Timestamps, so expectRoundtrip can't be used
143-
// here.
144-
const doc = db.collection('rooms').doc();
145-
return doc
146-
.set({ date: dateValue })
147-
.then(() => doc.get())
148-
.then(snapshot => {
149-
expect(snapshot.data()).to.deep.equal({
150-
date: Timestamp.fromDate(dateValue)
151-
});
152-
});
138+
return withTestDb(persistence, async db => {
139+
const date = new Date('2017-04-10T09:10:11.123Z');
140+
// Dates are returned as Timestamps
141+
const data = { date };
142+
const expectedData = { date: Timestamp.fromDate(date) };
143+
144+
await expectRoundtrip(
145+
db,
146+
data,
147+
/* validateSnapshot= */ true,
148+
expectedData
149+
);
153150
});
154151
});
155152

156153
it('can read and write timestamp fields', () => {
157-
return withTestDb(persistence, db => {
154+
return withTestDb(persistence, async db => {
158155
const timestampValue = Timestamp.now();
159-
return expectRoundtrip(db, { timestamp: timestampValue });
156+
await expectRoundtrip(db, { timestamp: timestampValue });
160157
});
161158
});
162159

163160
it('can read and write document references', () => {
164-
return withTestDoc(persistence, doc => {
165-
return expectRoundtrip(doc.firestore, { a: 42, ref: doc });
161+
return withTestDoc(persistence, async doc => {
162+
await expectRoundtrip(doc.firestore, { a: 42, ref: doc });
166163
});
167164
});
168165

169166
it('can read and write document references in an array', () => {
170-
return withTestDoc(persistence, doc => {
171-
return expectRoundtrip(doc.firestore, { a: 42, refs: [doc] });
167+
return withTestDoc(persistence, async doc => {
168+
await expectRoundtrip(doc.firestore, { a: 42, refs: [doc] });
172169
});
173170
});
174171
});

‎packages/firestore/test/unit/model/values.test.ts

+39-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import {
2525
valueCompare,
2626
valueEquals,
2727
estimateByteSize,
28-
refValue
28+
refValue,
29+
deepClone
2930
} from '../../../src/model/values';
3031
import * as api from '../../../src/protos/firestore_proto_api';
3132
import { primitiveComparator } from '../../../src/util/misc';
@@ -405,4 +406,41 @@ describe('Values', () => {
405406
)
406407
).to.equal('{a:1,b:2,c:3}');
407408
});
409+
410+
it('clones properties without normalization', () => {
411+
const values = [
412+
{ integerValue: '1' },
413+
{ integerValue: 1 },
414+
{ doubleValue: '2' },
415+
{ doubleValue: 2 },
416+
{ timestampValue: '2007-04-05T14:30:01Z' },
417+
{ timestampValue: { seconds: 1175783401 } },
418+
{ timestampValue: '2007-04-05T14:30:01.999Z' },
419+
{
420+
timestampValue: { seconds: 1175783401, nanos: 999000000 }
421+
},
422+
{ timestampValue: '2007-04-05T14:30:02Z' },
423+
{ timestampValue: { seconds: 1175783402 } },
424+
{ timestampValue: '2007-04-05T14:30:02.100Z' },
425+
{
426+
timestampValue: { seconds: 1175783402, nanos: 100000000 }
427+
},
428+
{ timestampValue: '2007-04-05T14:30:02.100001Z' },
429+
{
430+
timestampValue: { seconds: 1175783402, nanos: 100001000 }
431+
},
432+
{ bytesValue: new Uint8Array([0, 1, 2]) },
433+
{ bytesValue: 'AAEC' },
434+
{ bytesValue: new Uint8Array([0, 1, 3]) },
435+
{ bytesValue: 'AAED' }
436+
];
437+
438+
for (const value of values) {
439+
expect(deepClone(value)).to.deep.equal(value);
440+
const mapValue = { mapValue: { fields: { foo: value } } };
441+
expect(deepClone(mapValue)).to.deep.equal(mapValue);
442+
const arrayValue = { arrayValue: { values: [value] } };
443+
expect(deepClone(arrayValue)).to.deep.equal(arrayValue);
444+
}
445+
});
408446
});

0 commit comments

Comments
 (0)