Skip to content
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

feat: add react native web support #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Robert27
Copy link
Contributor

Added support for react native web by adapting the dispatcher behavior from aptabase-js.

  • Created two new dispatcher classes to handle dispatching for each platform.
  • Updated various files to handle the new platform
  • Updated a few dependencies
  • Added and updated the vitest tests. Although vitest does not support react-native, that is why every test file fails if a react native import is present in the parent. :/

@ivnbogdan
Resolves #12

@Robert27 Robert27 force-pushed the feat/react-native-web branch from c309c37 to bd56e18 Compare December 16, 2024 17:32
@ivnbogdan
Copy link
Contributor

@Robert27 thank you for your contribution.

Can you please highlight in a comment here the differences in implementation between the two and the reasoning for explicit support for React Native Web?

@Robert27
Copy link
Contributor Author

React Native Web has become increasingly popular, especially since Expo Web. The Bluesky web app, for example, is written with React Native Web. Specifically, I need this feature for my team, and I read the request in the discord. However, this SDK does not yet support the web platform.

Implementing it turned out to be a bit more complicated, as different philosophies were chosen between native and web.
I analyzed the other SDKs and used a comment from the founder in Discord. So web sends the events immediately, because the browser can be closed at any time, but native on the other hand batched to another endpoint, which also has no CORS restrictions.
Consequently, I had to create two dispatchers, one for web to the single event endpoint without user agent, so that CORS works, and the other one is the unmodified native dispatcher

}
return ["iOS", Platform.Version];
default:
return ["", ""];
Copy link

Choose a reason for hiding this comment

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

Maybe we can add web client here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remember correctly I left this empty as the server auto generates the operating system using the User Agent.
I'll double check it later.

Copy link

Choose a reason for hiding this comment

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

sorry, wrong place for a comment, I meant just to add another case in switch and return web client information

@beshkin
Copy link

beshkin commented Jan 23, 2025

I can absolutely relate on this PR.
@Robert27 great job. I checked the implementation and looks like exactly what we need

Comment on lines +24 to +27
const dispatcher =
Platform.OS === "web"
? new WebEventDispatcher(appKey, baseUrl, env)
: new NativeEventDispatcher(appKey, baseUrl, env);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we've had trouble in the past with automatically detecting environments
(like in aptabase/aptabase-swift#24 aptabase/aptabase-swift#27 )

I'm a bit reluctant to go this same path.

@Robert27 what are your thoughts and enabling the WebEventDispatcher only if a new, explicit option is set in options: AptabaseOptions

Would avoid breaking existing usages that simply upgrade, and would enable new integrations to explicitly opt in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for react native web
3 participants