-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[android][file-system] Migrate to new modules API #22728
[android][file-system] Migrate to new modules API #22728
Conversation
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.
mostly looks good. thanks for helping the large module migration.
note that this pr will break expo go building. would you like to include the fix for me?
diff --git a/android/expoview/src/main/java/host/exp/exponent/experience/DetachedModuleRegistryAdapter.kt b/android/expoview/src/main/java/host/exp/exponent/experience/DetachedModuleRegistryAdapter.kt
index 4f73ebfa31..e6eeb1a963 100644
--- a/android/expoview/src/main/java/host/exp/exponent/experience/DetachedModuleRegistryAdapter.kt
+++ b/android/expoview/src/main/java/host/exp/exponent/experience/DetachedModuleRegistryAdapter.kt
@@ -49,9 +49,6 @@ open class DetachedModuleRegistryAdapter(moduleRegistryProvider: ReactModuleRegi
// Overriding ScopedUIManagerModuleWrapper from ReactAdapterPackage
moduleRegistry.registerInternalModule(ScopedUIManagerModuleWrapper(reactContext))
- // Overriding expo-file-system FileSystemModule
- moduleRegistry.registerExportedModule(ScopedFileSystemModule(scopedContext))
-
// Overriding expo-secure-store
moduleRegistry.registerExportedModule(ScopedSecureStoreModule(scopedContext))
diff --git a/android/expoview/src/main/java/versioned/host/exp/exponent/ExperiencePackagePicker.kt b/android/expoview/src/main/java/versioned/host/exp/exponent/ExperiencePackagePicker.kt
index f31a8faa25..f62623aed3 100644
--- a/android/expoview/src/main/java/versioned/host/exp/exponent/ExperiencePackagePicker.kt
+++ b/android/expoview/src/main/java/versioned/host/exp/exponent/ExperiencePackagePicker.kt
@@ -22,6 +22,7 @@ import expo.modules.documentpicker.DocumentPickerModule
import expo.modules.easclient.EASClientModule
import expo.modules.print.PrintModule
import expo.modules.facedetector.FaceDetectorPackage
+import expo.modules.filesystem.FileSystemModule
import expo.modules.filesystem.FileSystemPackage
import expo.modules.font.FontLoaderPackage
import expo.modules.gl.GLPackage
@@ -120,6 +121,7 @@ object ExperiencePackagePicker : ModulesProvider {
DeviceModule::class.java,
DocumentPickerModule::class.java,
EASClientModule::class.java,
+ FileSystemModule::class.java,
PrintModule::class.java,
GLViewModule::class.java,
HapticsModule::class.java,
diff --git a/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ExpoModuleRegistryAdapter.kt b/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ExpoModuleRegistryAdapter.kt
index 7ecefe9ee7..310feda1b2 100644
--- a/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ExpoModuleRegistryAdapter.kt
+++ b/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ExpoModuleRegistryAdapter.kt
@@ -43,9 +43,6 @@ open class ExpoModuleRegistryAdapter(moduleRegistryProvider: ReactModuleRegistry
// Overriding expo-file-system FilePermissionModule
moduleRegistry.registerInternalModule(ScopedFilePermissionModule(scopedContext))
- // Overriding expo-file-system FileSystemModule
- moduleRegistry.registerExportedModule(ScopedFileSystemModule(scopedContext))
-
// Overriding expo-permissions ScopedPermissionsService
moduleRegistry.registerInternalModule(ScopedPermissionsService(scopedContext, experienceKey))
diff --git a/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ScopedFileSystemModule.kt b/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ScopedFileSystemModule.kt
deleted file mode 100644
index eb0913fab9..0000000000
--- a/android/expoview/src/main/java/versioned/host/exp/exponent/modules/universal/ScopedFileSystemModule.kt
+++ /dev/null
@@ -1,39 +0,0 @@
-package versioned.host.exp.exponent.modules.universal
-
-import android.content.Context
-import expo.modules.filesystem.FileSystemModule
-import host.exp.exponent.Constants
-import org.apache.commons.io.IOUtils
-import org.json.JSONObject
-import java.lang.Exception
-
-private const val SHELL_APP_EMBEDDED_MANIFEST_PATH = "shell-app-manifest.json"
-
-class ScopedFileSystemModule(context: Context) : FileSystemModule(context) {
- override fun getConstants(): Map<String, Any?> {
- return super.getConstants().toMutableMap().apply {
- this["bundledAssets"] = getBundledAssets()
- }
- }
-
- private fun getBundledAssets(): List<String>? {
- return if (!Constants.isStandaloneApp()) {
- // Fastpath, only standalone apps support bundled assets.
- null
- } else try {
- context.assets.open(SHELL_APP_EMBEDDED_MANIFEST_PATH).use {
- val jsonString = IOUtils.toString(it)
- val manifest = JSONObject(jsonString)
- val bundledAssetsJSON = manifest.getJSONArray("bundledAssets") ?: return null
- mutableListOf<String>().apply {
- for (i in 0 until bundledAssetsJSON.length()) {
- add(bundledAssetsJSON.getString(i))
- }
- }
- }
- } catch (ex: Exception) {
- ex.printStackTrace()
- null
- }
- }
-}
@brentvatne the ScopedFileSystemModule is only used for the deprecated standalone build. since we don't support standalone build anymore. we could remove the ScopedFileSystemModule entirely. this fix will change the bundledAssets
on expo go slightly, from undefined to null.
packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt
Show resolved
Hide resolved
packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt
Outdated
Show resolved
Hide resolved
No problem @Kudo 👍 |
android/expoview/src/main/java/versioned/host/exp/exponent/ExperiencePackagePicker.kt
Outdated
Show resolved
Hide resolved
packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.kt
Outdated
Show resolved
Hide resolved
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.
good to ship this 🚀. thanks @alanjhughes for this awesome work again 👏
Why
Continue migration to new modules api
How
Followed typical migration steps
Test Plan
Tested in bare expo. All tests ✅