Skip to content

[Fix] React Native Polyfill Load Race #199

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

Merged
merged 8 commits into from
Jun 5, 2024
Merged

Conversation

stevensJourney
Copy link
Collaborator

@stevensJourney stevensJourney commented Jun 4, 2024

Overview

The web socket release moved much of the shared sync stream fetch implementation to the common package. This used cross-fetch to provide a fetch implementation across the different SDKs. The release also made it possible to provide a fetchImplementation for AbstractRemote to use.

An issue could occur due to the nature of the DEFAULT_REMOTE_OPTIONS.fetchImplementation obtaining a reference to fetch in the global scope: i.e once the file has been loaded or imported. This can cause race conditions between applying fetch polyfills and obtaining the reference to fetch.

In the case of React Native this causes the timing of calling import 'react-native-polyfills-globals/auto' to be critical. It must be called before importing the PowerSync SDK.

This PR moves away from obtaining a reference to the default fetch implementation and introduces a FetchImplementationProvider lazy loader which will get the implementation when it is used. This should help in the case where polyfills are applied.

The concept of custom fetch implementations is then taken one step further in the React Native SDK by using the fetch implementation directly from react-native-fetch-api. Polyfills are still needed for ReadableStream, TextEncoder etc, but this ensures a fetch implementation which supports streaming will always be used in the React Native SDK.

Testing

This was tested in the React Native Supabase Todolist app on iOS with the HTTP streaming connection mode enabled.

Note: the Android version of the app is currently completely broken. Attempting to pnpm android results in
image
This issue seems to be common, but none of those suggestions fixes it. This PR includes updates for expo install --fix which also does not fix Android.

Testing Update:

It seems like the React Native Supabase todolist Expo 51 configuration broke sometime after it was introduced here #167.

The error above seems to be due to the native android project using the wrong version of expo-camera from the monorepo. Copying the demo to a standalone folder fixed the compilation issue.

The React native demos are now all updated to Expo 51 to ensure there are no package mismatches in the monorepo.

The fetch implementation changes were tested below.

Supabase Todolist app:

fetch_method_test.mov

Group chat app:

group_chat.mov

The Django app was also updated and verified that the app compiled and ran on both Android and iOS. This app was not end-to-end tested due to it's complex setup instructions.

@stevensJourney stevensJourney changed the title [Fix] React Native Polyfil Load Race [Fix] React Native Polyfill Load Race Jun 5, 2024
@stevensJourney stevensJourney marked this pull request as ready for review June 5, 2024 11:53
@stevensJourney stevensJourney merged commit 820a81d into main Jun 5, 2024
2 checks passed
@stevensJourney stevensJourney deleted the fix/react-native-fetch branch June 5, 2024 13:31
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.

3 participants