Skip to content

Enable strictPropertyInitialization for Firestore #2078

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 16 commits into from
Aug 12, 2019
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/api/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
private tokenListener: ((token: string | null) => void) | null = null;

/** Tracks the current User. */
private currentUser: User;
private currentUser: User = User.UNAUTHENTICATED;

/**
* Counter used to detect if the token changed while a getToken request was
Expand Down
95 changes: 42 additions & 53 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,24 +280,18 @@ class FirestoreSettings {
}
}

class FirestoreConfig {
databaseId: DatabaseId;
persistenceKey: string;
credentials: CredentialsProvider;
firebaseApp: FirebaseApp;
settings: FirestoreSettings;
persistence: boolean;
}

/**
* The root reference to the database.
*/
export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
// The objects that are a part of this API are exposed to third-parties as
// compiled javascript so we want to flag our private members with a leading
// underscore to discourage their use.
private readonly _config: FirestoreConfig;
readonly _databaseId: DatabaseId;
private readonly _persistenceKey: string;
private _credentials: CredentialsProvider;
private readonly _firebaseApp: FirebaseApp | null = null;
private _settings: FirestoreSettings;

// The firestore client instance. This will be available as soon as
// configureClient is called, but any calls against it will block until
Expand All @@ -311,18 +305,17 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
// TODO(mikelehen): Use modularized initialization instead.
readonly _queue = new AsyncQueue();

_dataConverter: UserDataConverter;
readonly _dataConverter: UserDataConverter;

constructor(databaseIdOrApp: FirestoreDatabase | FirebaseApp) {
const config = new FirestoreConfig();
if (typeof (databaseIdOrApp as FirebaseApp).options === 'object') {
// This is very likely a Firebase app object
// TODO(b/34177605): Can we somehow use instanceof?
const app = databaseIdOrApp as FirebaseApp;
config.firebaseApp = app;
config.databaseId = Firestore.databaseIdFromApp(app);
config.persistenceKey = config.firebaseApp.name;
config.credentials = new FirebaseCredentialsProvider(app);
this._firebaseApp = app;
this._databaseId = Firestore.databaseIdFromApp(app);
this._persistenceKey = app.name;
this._credentials = new FirebaseCredentialsProvider(app);
} else {
const external = databaseIdOrApp as FirestoreDatabase;
if (!external.projectId) {
Expand All @@ -332,15 +325,14 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
);
}

config.databaseId = new DatabaseId(external.projectId, external.database);
this._databaseId = new DatabaseId(external.projectId, external.database);
// Use a default persistenceKey that lines up with FirebaseApp.
config.persistenceKey = '[DEFAULT]';
config.credentials = new EmptyCredentialsProvider();
this._persistenceKey = '[DEFAULT]';
this._credentials = new EmptyCredentialsProvider();
}

config.settings = new FirestoreSettings({});
this._config = config;
this._databaseId = config.databaseId;
this._settings = new FirestoreSettings({});
this._dataConverter = this.createDataConverter(this._databaseId);
}

settings(settingsLiteral: firestore.Settings): void {
Expand All @@ -358,7 +350,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
}

const newSettings = new FirestoreSettings(settingsLiteral);
if (this._firestoreClient && !this._config.settings.isEqual(newSettings)) {
if (this._firestoreClient && !this._settings.isEqual(newSettings)) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
'Firestore has already been started and its settings can no longer ' +
Expand All @@ -367,11 +359,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
);
}

this._config.settings = newSettings;
this._settings = newSettings;
if (newSettings.credentials !== undefined) {
this._config.credentials = makeCredentialsProvider(
newSettings.credentials
);
this._credentials = makeCredentialsProvider(newSettings.credentials);
}
}

Expand Down Expand Up @@ -416,7 +406,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {

return this.configureClient(
new IndexedDbPersistenceSettings(
this._config.settings.cacheSizeBytes,
this._settings.cacheSizeBytes,
synchronizeTabs
)
);
Expand Down Expand Up @@ -489,30 +479,38 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {

private makeDatabaseInfo(): DatabaseInfo {
return new DatabaseInfo(
this._config.databaseId,
this._config.persistenceKey,
this._config.settings.host,
this._config.settings.ssl,
this._config.settings.forceLongPolling
this._databaseId,
this._persistenceKey,
this._settings.host,
this._settings.ssl,
this._settings.forceLongPolling
);
}

private configureClient(
persistenceSettings: InternalPersistenceSettings
): Promise<void> {
assert(
!!this._config.settings.host,
'FirestoreSettings.host cannot be falsey'
);
assert(!!this._settings.host, 'FirestoreSettings.host is not set');

assert(!this._firestoreClient, 'configureClient() called multiple times');

const databaseInfo = this.makeDatabaseInfo();

this._firestoreClient = new FirestoreClient(
PlatformSupport.getPlatform(),
databaseInfo,
this._credentials,
this._queue
);

return this._firestoreClient.start(persistenceSettings);
}

private createDataConverter(databaseId: DatabaseId): UserDataConverter {
const preConverter = (value: unknown): unknown => {
if (value instanceof DocumentReference) {
const thisDb = this._config.databaseId;
const otherDb = value.firestore._config.databaseId;
const thisDb = databaseId;
const otherDb = value.firestore._databaseId;
if (!otherDb.isEqual(thisDb)) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
Expand All @@ -521,21 +519,12 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
`for database ${thisDb.projectId}/${thisDb.database}`
);
}
return new DocumentKeyReference(this._config.databaseId, value._key);
return new DocumentKeyReference(databaseId, value._key);
} else {
return value;
}
};
this._dataConverter = new UserDataConverter(preConverter);

this._firestoreClient = new FirestoreClient(
PlatformSupport.getPlatform(),
databaseInfo,
this._config.credentials,
this._queue
);

return this._firestoreClient.start(persistenceSettings);
return new UserDataConverter(preConverter);
}

private static databaseIdFromApp(app: FirebaseApp): DatabaseId {
Expand All @@ -558,14 +547,14 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
}

get app(): FirebaseApp {
if (!this._config.firebaseApp) {
if (!this._firebaseApp) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
"Firestore was not initialized using the Firebase SDK. 'app' is " +
'not available'
);
}
return this._config.firebaseApp;
return this._firebaseApp;
}

INTERNAL = {
Expand Down Expand Up @@ -668,7 +657,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
// Note: this is not a property because the minifier can't work correctly with
// the way TypeScript compiler outputs properties.
_areTimestampsInSnapshotsEnabled(): boolean {
return this._config.settings.timestampsInSnapshots;
return this._settings.timestampsInSnapshots;
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/firestore/src/core/event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import { DocumentViewChange, ChangeType, ViewSnapshot } from './view_snapshot';
* tracked by EventManager.
*/
class QueryListenersInfo {
viewSnap: ViewSnapshot | null;
targetId: TargetId;
viewSnap: ViewSnapshot | null = null;
targetId: TargetId = 0;
listeners: QueryListener[] = [];
}

Expand Down Expand Up @@ -166,7 +166,7 @@ export class QueryListener {

private options: ListenOptions;

private snap: ViewSnapshot;
private snap: ViewSnapshot | null = null;

private onlineState: OnlineState = OnlineState.Unknown;

Expand Down
16 changes: 8 additions & 8 deletions packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,18 @@ export class FirestoreClient {
// initialization completes before any other work is queued, we're cheating
// with the types rather than littering the code with '!' or unnecessary
// undefined checks.
private eventMgr: EventManager;
private persistence: Persistence;
private localStore: LocalStore;
private remoteStore: RemoteStore;
private syncEngine: SyncEngine;
private eventMgr!: EventManager;
private persistence!: Persistence;
private localStore!: LocalStore;
private remoteStore!: RemoteStore;
private syncEngine!: SyncEngine;
// PORTING NOTE: SharedClientState is only used for multi-tab web.
private sharedClientState!: SharedClientState;

private lruScheduler?: LruScheduler;

private readonly clientId = AutoId.newId();

// PORTING NOTE: SharedClientState is only used for multi-tab web.
private sharedClientState: SharedClientState;

constructor(
private platform: Platform,
private databaseInfo: DatabaseInfo,
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/core/sync_engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class LimboResolution {
* decide whether it needs to manufacture a delete event for the target once
* the target is CURRENT.
*/
receivedDocument: boolean;
receivedDocument: boolean = false;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/core/target_id_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ enum GeneratorIds {
// TODO(mrschmidt): Explore removing this class in favor of generating these IDs
// directly in SyncEngine and LocalStore.
export class TargetIdGenerator {
private nextId: TargetId;
// Initialized in the constructor via call to seek().
private nextId!: TargetId;

/**
* Instantiates a new TargetIdGenerator. If a seed is provided, the generator
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/core/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class Transaction {
* A deferred usage error that occurred previously in this transaction that
* will cause the transaction to fail once it actually commits.
*/
private lastWriteError: FirestoreError;
private lastWriteError: FirestoreError | null = null;

/**
* Set of documents that have been written in the transaction.
Expand Down
17 changes: 11 additions & 6 deletions packages/firestore/src/local/indexeddb_persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,23 +236,28 @@ export class IndexedDbPersistence implements Persistence {
private readonly document: Document | null;
private readonly window: Window;

private simpleDb: SimpleDb;
// Technically these types should be `| undefined` because they are
// initialized asynchronously by start(), but that would be more misleading
// than useful.
private simpleDb!: SimpleDb;
private listenSequence!: ListenSequence;

private _started = false;
private isPrimary = false;
private networkEnabled = true;
private dbName: string;

/** Our window.unload handler, if registered. */
private windowUnloadHandler: (() => void) | null;
private windowUnloadHandler: (() => void) | null = null;
private inForeground = false;

private serializer: LocalSerializer;

/** Our 'visibilitychange' listener if registered. */
private documentVisibilityHandler: ((e?: Event) => void) | null;
private documentVisibilityHandler: ((e?: Event) => void) | null = null;

/** The client metadata refresh task. */
private clientMetadataRefresher: CancelablePromise<void>;
private clientMetadataRefresher: CancelablePromise<void> | null = null;

/** The last time we garbage collected the Remote Document Changelog. */
private lastGarbageCollectionTime = Number.NEGATIVE_INFINITY;
Expand All @@ -267,7 +272,6 @@ export class IndexedDbPersistence implements Persistence {
private readonly indexManager: IndexedDbIndexManager;
private readonly remoteDocumentCache: IndexedDbRemoteDocumentCache;
private readonly webStorage: Storage;
private listenSequence: ListenSequence;
readonly referenceDelegate: IndexedDbLruDelegate;

// Note that `multiClientParams` must be present to enable multi-client support while multi-tab
Expand Down Expand Up @@ -692,6 +696,7 @@ export class IndexedDbPersistence implements Persistence {
this.markClientZombied();
if (this.clientMetadataRefresher) {
this.clientMetadataRefresher.cancel();
this.clientMetadataRefresher = null;
}
this.detachVisibilityHandler();
this.detachWindowUnloadHook();
Expand Down Expand Up @@ -1131,7 +1136,7 @@ function clientMetadataStore(

/** Provides LRU functionality for IndexedDB persistence. */
export class IndexedDbLruDelegate implements ReferenceDelegate, LruDelegate {
private inMemoryPins: ReferenceSet | null;
private inMemoryPins: ReferenceSet | null = null;

readonly garbageCollector: LruGarbageCollector;

Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/local/lru_garbage_collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ const REGULAR_GC_DELAY_MS = 5 * 60 * 1000;
* whether or not GC is enabled, as well as which delay to use before the next run.
*/
export class LruScheduler {
private hasRun: boolean;
private hasRun: boolean = false;
private gcTask: CancelablePromise<void> | null;

constructor(
Expand Down
Loading