-
Notifications
You must be signed in to change notification settings - Fork 934
sdk 10.3.0 changed firestore webchannel to use xhr instead of fetch #7581
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
Comments
@atsjo Thanks for the report! I've taken a quick look — I think with #7569, it is expected that all POST (client -> server) request will now be using XHR, but the GET (server -> client streaming) requests should still be using Fetch. (This was the original intent of the Is that inline with what you have observed also? Disclaimer: I'm not familiar with Firestore requirements overall so i'm not sure if this is a breaking issue for anyone. I'll let members of the Firebase team comment later. @wu-hui FYI |
Note the fix does what @sampajano described in #7581 (comment) "all POST (client -> server) request will now be using XHR, but the GET (server -> client streaming) requests should still be using Fetch." |
@wu-hui Can you elaborate on why it's bad if Firebase uses XHR versus Fetch? Is it just that the XHR API is deprecated? |
Yeah, pretty much. I don't think XHR vs Fetch with requests makes much difference, it's more about we want to use newer APIs as long as it makes sense. Using Fetch both directions with webchannel's bidi-streams is under-tested however, so we will stay on the default configuration for now, and maybe flip to that later. Again, I don't think there is much to gain here performance-wise. |
Tried with 10.3.1, I assume the fix is rolled out there, but its still uses xhr for everything... (both POST and GET). I also observe typically one failed GET when the app starts up (as shown in the screenshot from dev-tools above). I don't care if its using xhr or fetch, just found it strange flipping everything back to xhr after running with fetch for years, with the changelog only mentioning that it changes how it enables fetch... The undocumented useFetchStreams also stopped working, and after running with fetch for years, I would assume using xhr for everything is under-tested, as mentioned above... |
The fix is not out with 10.3.1, should be the one after this. |
Verified canary build working as expected, using fetch for web-channel GET requests:-) |
Operating System
Windows 11
Browser Version
Chrome/Edge latest
Firebase SDK Version
10.3.0
Firebase SDK Product:
Firestore
Describe your project's tooling
Angular app using esbuild builder
Describe the problem
In sdk 10.2.0 fetch is used for firestore webchannel (I can se that from network tab in devtools, I think it's been used since 9.0), but 10.3.0 changed firestore webchannel to use xhr instead.
Overriding via undocumented useFetchStreams param in initializeFirestore doesn't work anymore either, so guessing this is a bug, and related to #7569. This is supposed to "fix how we enable fetch streams" to correct a bug, but seems to completely disable it. I can observe more aborted/cancelled communication in devtools with xhr, but it's still working...
Steps and code to reproduce issue
upgrade to 10.3.0 and observe communication via devtools
The text was updated successfully, but these errors were encountered: