Skip to content

Comment out multi-tab flag #1039

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.

Sign up for GitHub

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 4 commits into from
Jul 25, 2018
Merged

Comment out multi-tab flag #1039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
036659f
Comment out multi-tab flag
schmidt-sebastian Jul 24, 2018
f73452a
[AUTOMATED]: Prettier Code Styling
schmidt-sebastian Jul 24, 2018
c8c0bdb
No longer using PersistenceSettings in database.ts
schmidt-sebastian Jul 24, 2018
4c86c5b
Fix tag in TODO
schmidt-sebastian Jul 25, 2018
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
44 changes: 44 additions & 0 deletions packages/firebase/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,27 @@ declare namespace firebase.firestore {
timestampsInSnapshots?: boolean;
}

// TODO(multitab): Uncomment when multi-tab is released publicly.
// /**
// * Settings that can be passed to Firestore.enablePersistence() to configure
// * Firestore persistence.
// */
// export interface PersistenceSettings {
// /**
// * Whether to synchronize the in-memory state of multiple tabs. Setting this
// * to 'true' in all open tabs enables shared access to local persistence,
// * shared execution of queries and latency-compensated local document updates
// * across all connected instances.
// *
// * To enable this mode, `experimentalTabSynchronization:true` needs to be set
// * globally in all active tabs. If omitted or set to 'false',
// * `enablePersistence()` will fail in all but the first tab.
// *
// * NOTE: This mode is not yet recommended for production use.
// */
// experimentalTabSynchronization?: boolean;
// }

export type LogLevel = 'debug' | 'error' | 'silent';

export function setLogLevel(logLevel: LogLevel): void;
Expand Down Expand Up @@ -808,6 +829,29 @@ declare namespace firebase.firestore {
*/
enablePersistence(): Promise<void>;

// TODO(multitab): Uncomment when multi-tab is released publicly.
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we'll do the right thing, but really this should be "Uncomment (and remove above version) when. .."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take care of that when the day comes :)

// /**
// * Attempts to enable persistent storage, if possible.
// *
// * Must be called before any other methods (other than settings()).
// *
// * If this fails, enablePersistence() will reject the promise it returns.
// * Note that even after this failure, the firestore instance will remain
// * usable, however offline persistence will be disabled.
// *
// * There are several reasons why this can fail, which can be identified by
// * the `code` on the error.
// *
// * * failed-precondition: The app is already open in another browser tab.
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this should probably have some mention of experimentalTabSynchronization... but since it's experimental maybe we don't care to give it too much verbiage in this method description...

// * * unimplemented: The browser is incompatible with the offline
// * persistence implementation.
// *
// * @param settings Optional settings object to configure persistence.
// * @return A promise that represents successfully enabling persistent
// * storage.
// */
// enablePersistence(settings?: PersistenceSettings): Promise<void>;

/**
* Gets a `CollectionReference` instance that refers to the collection at
* the specified path.
Expand Down
65 changes: 44 additions & 21 deletions packages/firestore-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,26 @@ export interface Settings {
timestampsInSnapshots?: boolean;
}

/**
* Settings that can be passed to Firestore.enablePersistence() to configure
* Firestore persistence.
*/
export interface PersistenceSettings {
/**
* Whether to synchronize the in-memory state of multiple tabs. Setting this
* to 'true' in all open tabs enables shared access to local persistence,
* shared execution of queries and latency-compensated local document updates
* across all connected instances.
*
* To enable this mode, `experimentalTabSynchronization:true` needs to be set
* globally in all active tabs. If omitted or set to 'false',
* `enablePersistence()` will fail in all but the first tab.
*
* NOTE: This mode is not yet recommended for production use.
*/
experimentalTabSynchronization?: boolean;
}
// TODO(multitab): Uncomment when multi-tab is released publicly.
// /**
// * Settings that can be passed to Firestore.enablePersistence() to configure
// * Firestore persistence.
// */
// export interface PersistenceSettings {
// /**
// * Whether to synchronize the in-memory state of multiple tabs. Setting this
// * to 'true' in all open tabs enables shared access to local persistence,
// * shared execution of queries and latency-compensated local document updates
// * across all connected instances.
// *
// * To enable this mode, `experimentalTabSynchronization:true` needs to be set
// * globally in all active tabs. If omitted or set to 'false',
// * `enablePersistence()` will fail in all but the first tab.
// *
// * NOTE: This mode is not yet recommended for production use.
// */
// experimentalTabSynchronization?: boolean;
// }

export type LogLevel = 'debug' | 'error' | 'silent';

Expand Down Expand Up @@ -111,11 +112,33 @@ export class FirebaseFirestore {
* * unimplemented: The browser is incompatible with the offline
* persistence implementation.
*
* @param settings Optional settings object to configure persistence.
* @return A promise that represents successfully enabling persistent
* storage.
*/
enablePersistence(settings?: PersistenceSettings): Promise<void>;
enablePersistence(): Promise<void>;

// TODO(multitab): Uncomment when multi-tab is released publicly.
// /**
// * Attempts to enable persistent storage, if possible.
// *
// * Must be called before any other methods (other than settings()).
// *
// * If this fails, enablePersistence() will reject the promise it returns.
// * Note that even after this failure, the firestore instance will remain
// * usable, however offline persistence will be disabled.
// *
// * There are several reasons why this can fail, which can be identified by
// * the `code` on the error.
// *
// * * failed-precondition: The app is already open in another browser tab.
// * * unimplemented: The browser is incompatible with the offline
// * persistence implementation.
// *
// * @param settings Optional settings object to configure persistence.
// * @return A promise that represents successfully enabling persistent
// * storage.
// */
// enablePersistence(settings?: PersistenceSettings): Promise<void>;

/**
* Gets a `CollectionReference` instance that refers to the collection at
Expand Down
11 changes: 6 additions & 5 deletions packages/firestore/src/api/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ class FirestoreConfig {
persistence: boolean;
}

// TODO(multitab): Replace with Firestore.PersistenceSettings
// tslint:disable-next-line:no-any The definition for these settings is private
export type _PersistenceSettings = any;

/**
* Encapsulates the settings that can be used to configure Firestore
* persistence.
Expand All @@ -210,10 +214,7 @@ export class PersistenceSettings {
/** Whether to enable multi-tab synchronization. */
experimentalTabSynchronization: boolean;

constructor(
readonly enabled: boolean,
settings?: firestore.PersistenceSettings
) {
constructor(readonly enabled: boolean, settings?: _PersistenceSettings) {
assert(
enabled || !settings,
'Can only provide PersistenceSettings with persistence enabled'
Expand Down Expand Up @@ -327,7 +328,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
return this._firestoreClient.disableNetwork();
}

enablePersistence(settings?: firestore.PersistenceSettings): Promise<void> {
enablePersistence(settings?: _PersistenceSettings): Promise<void> {
if (this._firestoreClient) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
Expand Down