-
Notifications
You must be signed in to change notification settings - Fork 944
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
Conversation
9ce9665
to
1e38673
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with questions
packages/firebase/index.d.ts
Outdated
* buffer traffic indefinitely. Use of this option will cause some | ||
* performance degradation though. | ||
* | ||
* This setting may be removed in a future release. |
There was a problem hiding this comment.
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.
packages/firestore/CHANGELOG.md
Outdated
@@ -1,4 +1,6 @@ | |||
# Unreleased | |||
- [feature] Added an `experimentalForceLongPolling` setting that may be helpful |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@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). |
No description provided.