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

[android][file-system] Migrate to new modules API #22728

Merged
merged 5 commits into from
Jun 7, 2023

Conversation

alanjhughes
Copy link
Collaborator

Why

Continue migration to new modules api

How

Followed typical migration steps

Test Plan

Tested in bare expo. All tests ✅

@alanjhughes alanjhughes changed the title [android][file-system] Migrate to modules API [android][file-system] Migrate to new modules API Jun 2, 2023
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jun 2, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jun 2, 2023
@alanjhughes alanjhughes marked this pull request as ready for review June 2, 2023 18:22
Copy link
Contributor

@Kudo Kudo left a 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.

@alanjhughes
Copy link
Collaborator Author

No problem @Kudo 👍

Copy link
Contributor

@Kudo Kudo left a 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 👏

@alanjhughes alanjhughes merged commit 4fde8ae into expo:main Jun 7, 2023
@alanjhughes alanjhughes deleted the @alanhughes/android/file-system branch June 7, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants