-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
fix(transport): Fallback to fetch transport if native not available #2695
Conversation
@@ -41,34 +38,16 @@ export class ReactNativeClient extends BaseClient<ReactNativeClientOptions> { | |||
* @param options Configuration options for this SDK. | |||
*/ | |||
public constructor(options: ReactNativeClientOptions) { | |||
if (!options.transport) { |
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.
option.transport
is required and has a fallback in sdk.tsx
, so this if statement wasn't working
Android (legacy) Performance metrics 🚀
|
Android (new) Performance metrics 🚀
|
iOS (new) Performance metrics 🚀
|
This is ready for review. |
* This is a workaround for now using fetch on RN, this is a known issue in react-native and only generates a warning | ||
* YellowBox deprecated and replaced with with LogBox in RN 0.63 | ||
*/ | ||
export function ignoreRequireCycleLogs(): void { |
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 agree with the refactoring here and separating the concerns.
iOS (legacy) Performance metrics 🚀
|
This is implemented in the init function in sdk.tsx
📢 Type of change
📜 Description
Fixes the unreachable fallback to the fetch transport.
Simplify client constructor. Make it more readable.
💡 Motivation and Context
Not working fetch transport.
💚 How did you test it?
unit tests
📝 Checklist
🔮 Next steps