Skip to content

Add "experimentalForceLongPolling" option. #1662

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 3 commits into from
Apr 9, 2019

Conversation

mikelehen
Copy link
Contributor

No description provided.

@mikelehen mikelehen force-pushed the mikelehen/force-long-polling branch from 9ce9665 to 1e38673 Compare April 5, 2019 19:48
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM with questions

* buffer traffic indefinitely. Use of this option will cause some
* performance degradation though.
*
* This setting may be removed in a future release.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we invite users to discuss with us when they're using this?

The backend will have logs of when this is actually used, but if the rest of the world starts using this by default we won't know why otherwise.

@@ -1,4 +1,6 @@
# Unreleased
- [feature] Added an `experimentalForceLongPolling` setting that may be helpful
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make the call to action more concrete here?

Maybe "... setting that can be used to work around proxies that prevent the Firestore client from connecting to its backend"?

@wilhuff wilhuff assigned mikelehen and unassigned wilhuff Apr 8, 2019
@mikelehen mikelehen assigned wilhuff and unassigned mikelehen Apr 8, 2019
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

@mikelehen
Copy link
Contributor Author

@hsubox76 Can you approve for the .d.ts updates?

FWIW- This option is actually only applicable to web, and so if there's a way to hide it from the node docs, let me know. Else, it's not a big deal (you can actually set the option from node, it'll just be a no-op).

@mikelehen mikelehen requested a review from hsubox76 April 9, 2019 21:21
@mikelehen mikelehen merged commit a669ee5 into master Apr 9, 2019
@mikelehen mikelehen deleted the mikelehen/force-long-polling branch April 9, 2019 22:44
@firebase firebase locked and limited conversation to collaborators Oct 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants