Repository navigation
fix(react-native): let the app provide expo-file-system - #2097
Merged
Merged
Conversation
As a regular dependency pinned to 18, expo-file-system nested a second native copy in apps on Expo SDK 54 and later, crashing Android at launch, and from 19 the root legacy methods throw, breaking file uploads. No single pinned version works across Expo SDKs, so the app now supplies it as a peer and the SDK loads the legacy API from "expo-file-system/legacy" when it exists, falling back to the package root on 18 (Expo SDK 52 and 53). Android upload chunks written to the cache directory are now deleted once their request completes.
🟢 Tier S · Ready to mergeThis change makes
📂 Walkthrough · 14
🔇 Filtered out · 1Findings Hansi considered but did not post.
Reviewed |
3 tasks
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes one build of the React Native SDK work on Expo SDK 52 through 58.
The bug
The SDK lists
expo-file-system: "18.*.*"as a regular dependency.react-native-appwrite@2.0.0ships that, and it breaks newer Expo apps in two ways:NoClassDefFoundError: FilePermissionModule(App crashes at launch on Expo SDK 54: expo-file-system@18 pin conflicts with the app's expo-file-system 19 sdk-for-react-native#114).readAsStringAsync,writeAsStringAsyncand the rest of the old API throw. Onlyexpo-file-system/legacyworks, so uploads fail on Expo SDK 55+ when the root module resolves to the app's copy (🐛 Bug Report: Storage uploads fail on Expo SDK 55 because the SDK imports a removed FileSystem API sdk-for-react-native#112).18.x (Expo SDK 52 and 53) has no
/legacyentry, so no single pinned version or import path works across Expo SDKs. #1978 pinned^57with a/legacyimport and was reverted in #1979 because a regular dependency on 57 breaks apps on 53–56 the same way 18 breaks 54+.The fix
expo-file-systemmoves topeerDependenciesas>=18.0.0, so the app's own copy (the one matching its Expo SDK) is used and nothing gets nested. A58.0.9devDependency provides types for the build. Theexpopeer goes from*to>=52.0.0, matching the existingreact-native >=0.76.7floor.src/lib/file-system.tsrequiresexpo-file-system/legacyand falls back toexpo-file-systemif that fails. Because the/legacyrequire sits in atry, Metro treats it as optional: Expo's Metro config setsallowOptionalDependencies: true(checked in@expo/metro-config0.19, used by Expo SDK 52), so apps on 18 still bundle and use the root API. PostHog's React Native SDK does the same thing (OptionalExpoFileSystemLegacyfalling back toOptionalExpoFileSystem). The loader is typed against the legacy API./legacystill ships in expo-file-system 58, and Expo hasn't announced removing it.moduleResolution: "node",expo-file-system/legacyresolves to the package'slegacy.tssource rather than its declarations, andtscthen fails onexpo-modules-coreimports. The tsconfig now uses"bundler", which follows the package'sexportsmap tobuild/legacy/index.d.ts. This is how Metro (package exports on by default since Expo SDK 53) and Rollup resolve it as well.cacheDirectoryand never removed. Each one is now deleted once its request completes, whether it succeeded or failed.expo-file-system, and the README says Expo SDK 52+ is supported and thatnpx expo installpicks the matching version.Release
This should ship as a minor (2.1.0). Apps on Expo SDK 52 and 53 keep working on 18 through the fallback, and every Expo SDK already includes
expo-file-system, sonpx expo installadds the right version. The apps that need anything new are the ones on 54+ that crash or fail to upload today.Supersedes #1691 by @bhardwajparth51, who diagnosed the
/legacymove for Expo SDK 55. That PR's bare/legacyimport fixes 55+ but would break 52/53, where/legacydoesn't exist. This PR keeps that import and adds the fallback and the peer change.Test Plan
testNativeModulesArePeerDependencieschecks thatpackage.jsonand the root ofpackage-lock.jsondeclareexpo-file-system >=18.0.0andexpo >=52.0.0as peers, and that neither is listed independencies. Againstmain's templates it fails (package.json does not declare expo as a >=52.0.0 peer). With this change it passes, along with the rest of the Generation suite.tests/e2e/ReactNativeTest.php): the browser harness now models Metro's runtimerequirewith a module registry (shims/metro-require.js). The default layout is expo-file-system 19+:/legacyserves the working API and the root throws, as the real package does. The static-import aliases follow the same layout. After the existing uploads, the test switchesPlatform.OStoandroidand uploads the large file twice. The first run uses the 19+ layout. The second uses the 18 layout, which has no/legacyand keeps the API at the root. After each run the test prints how many temp chunk files are left, and expects0.main's templates and the new harness, the uploads go through the throwing root module and the test fails.OK (1 test, 274 assertions)).react-native client.npm ci --omit=peer,format:check,lint,analyseandbuildall pass. With--omit=peer, the types come from the devDependency.composer lint,composer lint-twig,composer refactor:checkand the.twigmax line length check all pass../scripts/update-lockfiles.sh react-native, cut down to the root entry and theexpo-file-systementry (18.1.11 → 58.0.9). The script also moved the dev-onlyexpotoolchain from 57 to 58, and that churn is left out.Related PRs and Issues
/legacy, reverted)Have you read the Contributing Guidelines on issues?
Yes.
Closes #2098