-
Notifications
You must be signed in to change notification settings - Fork 587
Fixed MongoDBCollection#watch
on React Native
#4155
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
@@ -6,7 +6,7 @@ | |||
"./header.eslintrc" | |||
], | |||
"env": { | |||
"es2017": true | |||
"es2020": true |
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.
This is needed to be able to use the globalThis
keyword and should be okay for the platforms we support.
@@ -63,6 +63,21 @@ export default [ | |||
nodeResolve(), | |||
], | |||
}, | |||
{ | |||
input: "src/react-native/index.ts", |
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.
We're adding a new "react-native" bundle to separate the code that runs on "react-native" from that which run on web.
// Setting this non-standard option to enable text streaming | ||
// See https://github.com/react-native-community/fetch#enable-text-streaming | ||
DefaultNetworkTransport.extraFetchOptions = { | ||
reactNative: { textStreaming: true }, |
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.
See https://github.com/react-native-community/fetch#enable-text-streaming for details on why we're adding this.
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.
Please add it as a comment
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.
It's already there - two lines above the line I commented on here :)
@@ -69,6 +70,7 @@ export class DefaultNetworkTransport implements NetworkTransport { | |||
try { | |||
// We'll await the response to catch throw our own error | |||
return await DefaultNetworkTransport.fetch(url, { | |||
...DefaultNetworkTransport.extraFetchOptions, |
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.
I tried setting this only on the streaming requests, but that yield the following in my test app:
LOG Running "ReactNativeTestApp" with {"rootTag":61,"initialProps":{}}
WARN Invalid responseType: blob
ERROR {"code": -1, "message": "undefined is not an object (evaluating 'options.blobId')"}
WARN Invalid responseType: blob
LOG Unable to symbolicate stack trace: undefined is not an object (evaluating 'options.blobId')
WARN Invalid responseType: blob
WARN Invalid responseType: blob
WARN Invalid responseType: blob
c985b88
to
460ab84
Compare
// Setting this non-standard option to enable text streaming | ||
// See https://github.com/react-native-community/fetch#enable-text-streaming | ||
DefaultNetworkTransport.extraFetchOptions = { | ||
reactNative: { textStreaming: true }, |
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.
Please add it as a comment
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.
Awesome. Do we need to prominently document this somewhere, besides in the comments?
Besides the changes I'm proposing to the TypeScript docs and JSDoc, I would say the instructions on adding the babel plugin and globals polyfill needs to be inserted on https://docs.mongodb.com/realm/sdk/react-native/examples/query-mongodb/#real-time-change-notifications as well. |
What, How & Why?
This closes #3494 by adding documentation on installing polyfills and a babel plugin, plus calling the React Native fetch polyfill with the right option.
If the user forgets to add the polyfill and calls
watch()
TypeScript documentation as seen in VC Code
The JSDoc documentation
☑️ ToDos
Compatibility
label is updated or copied from previous entryBreaking
label has been applied or is not necessaryIf this PR adds or changes public API's: