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

Port Compat tests to v9 API #5844

Merged
merged 11 commits into from Jan 12, 2022
Merged
47 changes: 12 additions & 35 deletions integration/firestore/firebase_export.ts
Expand Up @@ -15,10 +15,12 @@
* limitations under the License.
*/

import firebase from '@firebase/app-compat';
import '@firebase/firestore-compat';
import { FirebaseApp } from '@firebase/app-types';
import { Settings, FirebaseFirestore } from '@firebase/firestore-types';
import { FirebaseApp, initializeApp } from '@firebase/app';
import {
Firestore,
FirestoreSettings,
initializeFirestore
} from '@firebase/firestore';

// This file replaces "packages/firestore/test/integration/util/firebase_export"
// and depends on the minified sources.
Expand All @@ -28,43 +30,18 @@ let appCount = 0;
export function newTestFirestore(
projectId: string,
nameOrApp?: string | FirebaseApp,
settings?: Settings
): FirebaseFirestore {
settings?: FirestoreSettings
): Firestore {
if (nameOrApp === undefined) {
nameOrApp = 'test-app-' + appCount++;
}
const app =
typeof nameOrApp === 'string'
? firebase.initializeApp({ apiKey: 'fake-api-key', projectId }, nameOrApp)
? initializeApp({ apiKey: 'fake-api-key', projectId }, nameOrApp)
: nameOrApp;

const firestore = firebase.firestore(app);
if (settings) {
firestore.settings(settings);
}
return firestore;
return initializeFirestore(app, settings || {});
}

export function usesFunctionalApi(): false {
return false;
}

const Blob = firebase.firestore.Blob;
const DocumentReference = firebase.firestore.DocumentReference;
const FieldPath = firebase.firestore.FieldPath;
const FieldValue = firebase.firestore.FieldValue;
const Firestore = firebase.firestore.Firestore;
const GeoPoint = firebase.firestore.GeoPoint;
const QueryDocumentSnapshot = firebase.firestore.QueryDocumentSnapshot;
const Timestamp = firebase.firestore.Timestamp;
export * from '@firebase/firestore';

export {
Blob,
DocumentReference,
FieldPath,
FieldValue,
Firestore,
GeoPoint,
QueryDocumentSnapshot,
Timestamp
};
export type PrivateSettings = Record<string, any>;
27 changes: 10 additions & 17 deletions integration/firestore/gulpfile.js
Expand Up @@ -53,25 +53,18 @@ function copyTests() {
.pipe(
replace(
/**
* This regex is designed to match the following statement used in our
* firestore integration test suites:
*
* import * as firebaseExport from '../../util/firebase_export';
*
* It will handle variations in whitespace, single/double quote
* differences, as well as different paths to a valid firebase_export
* This regex is designed to match the Firebase import in our
* integration tests.
*/
/import\s+\* as firebaseExport\s+from\s+('|")[^\1]+firebase_export\1;?/,
`import * as firebaseExport from '${resolve(
__dirname,
'./firebase_export'
)}';
/\s+from '\.(\.\/util)?\/firebase_export';/,
` from '${resolve(__dirname, './firebase_export')}';
if (typeof process === 'undefined') {
process = { env: { INCLUDE_FIRESTORE_PERSISTENCE: '${isPersistenceEnabled()}' } } as any;
} else {
process.env.INCLUDE_FIRESTORE_PERSISTENCE = '${isPersistenceEnabled()}';
}`
if (typeof process === 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this indentation change intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the output looks much better. This is a search/replace thing and the output shouldn't be indented. It doesn't really matter though since no one should see this, but I like to go above and beyond :)

process = { env: { INCLUDE_FIRESTORE_PERSISTENCE: '${isPersistenceEnabled()}' } } as any;
} else {
process.env.INCLUDE_FIRESTORE_PERSISTENCE = '${isPersistenceEnabled()}';
}
`
)
)
.pipe(
Expand Down
3 changes: 2 additions & 1 deletion integration/firestore/package.json
Expand Up @@ -6,6 +6,7 @@
"build:deps": "lerna run --scope @firebase/'{app,firestore}' --include-dependencies build",
"build:persistence": "INCLUDE_FIRESTORE_PERSISTENCE=true gulp compile-tests",
"build:memory": "INCLUDE_FIRESTORE_PERSISTENCE=false gulp compile-tests",
"prettier": "prettier --write '*.js' '*.ts'",
"test": "yarn build:memory; karma start --single-run; yarn build:persistence; karma start --single-run;",
"test:ci": "node ../../scripts/run_tests_in_ci.js -s test",
"test:persistence": " yarn build:persistence; karma start --single-run",
Expand All @@ -15,7 +16,7 @@
},
"devDependencies": {
"@firebase/app": "0.7.12",
"@firebase/firestore-compat": "0.1.11",
"@firebase/firestore": "3.4.2",
"@types/mocha": "9.0.0",
"gulp": "4.0.2",
"gulp-filter": "7.0.0",
Expand Down
5 changes: 1 addition & 4 deletions packages/firestore/package.json
Expand Up @@ -8,7 +8,7 @@
"author": "Firebase <firebase-support@google.com> (https://firebase.google.com/)",
"scripts": {
"bundle": "rollup -c",
"prebuild": "yarn test:prepare && tsc --emitDeclarationOnly --declaration -p tsconfig.json; yarn api-report",
"prebuild": "tsc --emitDeclarationOnly --declaration -p tsconfig.json; yarn api-report",
"build": "run-p build:lite build:main",
"build:release": "yarn build && yarn typings:public",
"build:scripts": "tsc -moduleResolution node --module commonjs scripts/*.ts && ls scripts/*.js | xargs -I % sh -c 'terser % -o %'",
Expand All @@ -25,8 +25,6 @@
"test:lite:prod": "node ./scripts/run-tests.js --platform node_lite --main=lite/index.ts 'test/lite/**/*.test.ts'",
"test:lite:browser": "karma start --single-run --lite",
"test:lite:browser:debug": "karma start --browsers=Chrome --lite --auto-watch",
"pretest": "yarn test:prepare",
"pretest:ci": "yarn pretest",
"test": "run-s lint test:all",
"test:ci": "node ../../scripts/run_tests_in_ci.js -s test:all:ci",
"test:all:ci": "run-p test:browser test:lite:browser test:travis",
Expand All @@ -44,7 +42,6 @@
"api-report:api-json": "rm -rf temp && api-extractor run --local --verbose",
"api-report": "run-s api-report:main api-report:lite && yarn api-report:api-json",
"doc": "api-documenter markdown --input temp --output docs",
"test:prepare": "node ./scripts/prepare-test.js",
"typings:public": "node ../../scripts/build/use_typings.js ./dist/index.d.ts"
},
"exports": {
Expand Down
18 changes: 0 additions & 18 deletions packages/firestore/scripts/prepare-test.js

This file was deleted.

84 changes: 0 additions & 84 deletions packages/firestore/scripts/prepare-test.ts

This file was deleted.

12 changes: 6 additions & 6 deletions packages/firestore/src/api/snapshot.ts
Expand Up @@ -59,12 +59,12 @@ import { SnapshotListenOptions } from './reference_impl';
* }
*
* const postConverter = {
* toFirestore(post: WithFieldValue<Post>): firebase.firestore.DocumentData {
* toFirestore(post: WithFieldValue<Post>): DocumentData {
* return {title: post.title, author: post.author};
* },
* fromFirestore(
* snapshot: firebase.firestore.QueryDocumentSnapshot,
* options: firebase.firestore.SnapshotOptions
* snapshot: QueryDocumentSnapshot,
* options: SnapshotOptions
* ): Post {
* const data = snapshot.data(options)!;
* return new Post(data.title, data.author);
Expand Down Expand Up @@ -269,7 +269,7 @@ export class DocumentSnapshot<
* Retrieves all fields in the document as an `Object`. Returns `undefined` if
* the document doesn't exist.
*
* By default, `FieldValue.serverTimestamp()` values that have not yet been
* By default, `serverTimestamp()` values that have not yet been
* set to their final value will be returned as `null`. You can override
* this by passing an options object.
*
Expand Down Expand Up @@ -306,7 +306,7 @@ export class DocumentSnapshot<
* Retrieves the field specified by `fieldPath`. Returns `undefined` if the
* document or field doesn't exist.
*
* By default, a `FieldValue.serverTimestamp()` that has not yet been set to
* By default, a `serverTimestamp()` that has not yet been set to
* its final value will be returned as `null`. You can override this by
* passing an options object.
*
Expand Down Expand Up @@ -353,7 +353,7 @@ export class QueryDocumentSnapshot<
/**
* Retrieves all fields in the document as an `Object`.
*
* By default, `FieldValue.serverTimestamp()` values that have not yet been
* By default, `serverTimestamp()` values that have not yet been
* set to their final value will be returned as `null`. You can override
* this by passing an options object.
*
Expand Down
15 changes: 7 additions & 8 deletions packages/firestore/src/lite-api/query.ts
Expand Up @@ -480,8 +480,7 @@ export function newQueryFilter(
if (op === Operator.ARRAY_CONTAINS || op === Operator.ARRAY_CONTAINS_ANY) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid Query. You can't perform '${op}' ` +
'queries on FieldPath.documentId().'
`Invalid Query. You can't perform '${op}' queries on documentId().`
);
} else if (op === Operator.IN || op === Operator.NOT_IN) {
validateDisjunctiveFilterElements(value, op);
Expand Down Expand Up @@ -639,7 +638,7 @@ export function newQueryBoundFromFields(
if (!isCollectionGroupQuery(query) && rawValue.indexOf('/') !== -1) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` +
`Invalid query. When querying a collection and ordering by documentId(), ` +
`the value passed to ${methodName}() must be a plain document ID, but ` +
`'${rawValue}' contains a slash.`
);
Expand All @@ -649,7 +648,7 @@ export function newQueryBoundFromFields(
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying a collection group and ordering by ` +
`FieldPath.documentId(), the value passed to ${methodName}() must result in a ` +
`documentId(), the value passed to ${methodName}() must result in a ` +
`valid document path, but '${path}' is not because it contains an odd number ` +
`of segments.`
);
Expand Down Expand Up @@ -681,15 +680,15 @@ function parseDocumentIdValue(
if (documentIdValue === '') {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
'Invalid query. When querying with FieldPath.documentId(), you ' +
'Invalid query. When querying with documentId(), you ' +
'must provide a valid document ID, but it was an empty string.'
);
}
if (!isCollectionGroupQuery(query) && documentIdValue.indexOf('/') !== -1) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying a collection by ` +
`FieldPath.documentId(), you must provide a plain document ID, but ` +
`documentId(), you must provide a plain document ID, but ` +
`'${documentIdValue}' contains a '/' character.`
);
}
Expand All @@ -698,7 +697,7 @@ function parseDocumentIdValue(
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying a collection group by ` +
`FieldPath.documentId(), the value provided must result in a valid document path, ` +
`documentId(), the value provided must result in a valid document path, ` +
`but '${path}' is not because it has an odd number of segments (${path.length}).`
);
}
Expand All @@ -708,7 +707,7 @@ function parseDocumentIdValue(
} else {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` +
`Invalid query. When querying with documentId(), you must provide a valid ` +
`string or a DocumentReference, but it was: ` +
`${valueDescription(documentIdValue)}.`
);
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/src/lite-api/snapshot.ts
Expand Up @@ -57,10 +57,10 @@ import { AbstractUserDataWriter } from './user_data_writer';
* }
*
* const postConverter = {
* toFirestore(post: WithFieldValue<Post>): firebase.firestore.DocumentData {
* toFirestore(post: WithFieldValue<Post>): DocumentData {
* return {title: post.title, author: post.author};
* },
* fromFirestore(snapshot: firebase.firestore.QueryDocumentSnapshot): Post {
* fromFirestore(snapshot: QueryDocumentSnapshot): Post {
* const data = snapshot.data(options)!;
* return new Post(data.title, data.author);
* }
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/lite-api/user_data_reader.ts
Expand Up @@ -722,7 +722,7 @@ export function parseData(
validatePlainObject('Unsupported field value:', context, input);
return parseObject(input, context);
} else if (input instanceof FieldValue) {
// FieldValues usually parse into transforms (except FieldValue.delete())
// FieldValues usually parse into transforms (except deleteField())
// in which case we do not want to include this field in our parsed data
// (as doing so will overwrite the field directly prior to the transform
// trying to transform it). So we don't add this location to
Expand Down