Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor to support multi-package architecture #143

Merged
merged 10 commits into from
Feb 8, 2025

Conversation

MeLlamoPablo
Copy link
Contributor

Overview

This PR splits the multiple supported permissions across different packages, allowing users to only install the packages for the permissions they need. This comes with the upside that we won't reference unnecessary iOS libraries and we will no longer trigger AppStore Connect automated alerts. Even though those alerts can be disregarded, it feels wrong to do so, and they can be confusing (especially for newbie iOS devs like me!)

Fixes #103

Migration path

This PR introduces a breaking change, but it is pretty minimal. Developers must complete the following steps:

  1. Install the packages they need
  2. Import the Permission.xxx symbols

This should be pretty painless, as Android Studio features context actions for both steps.

Code walkthrough

This refactor introduces evolves the existing concept of iOS pemission delegates into multiplatform permission delegates. A "permission delegate" is now an object that contains
platform-specific logic to request the permission and retrieve its state. For iOS, this is platform code calling iOS-specific APIs. For Android, this is simply a list of Manifest.permission values.

The iOS code is pretty much untouched in this PR, as it already was structured in an isolated way. On the other hand, the Android code was moved from PermissionsControllerImpl into android-specific delegates.

A permission now contains a reference to its platform-dependant delegate:

internal expect val cameraDelegate: PermissionDelegate

object CameraPermission : Permission {
    override val delegate get() = cameraDelegate
}

The Permission enum is transformed into an empty object, and each permission-specific package extends it to ease the migration path:

val Permission.Companion.CAMERA get() = CameraPermission

Thank you @Alex009 for this brilliant suggestion!

Now, both Android's and iOS' PermissionController are not aware of which permissions exist anymore. Instead we pass a Permission object which contains the delegate, and the controller interacts with the delegate to request the permission or retrieve its state.

@MeLlamoPablo
Copy link
Contributor Author

I wonder if we don't need to split into multiple modules. Maybe it's enough to split into multiple packages that get distributed under the same artifact. When I have time I'll prototype this and update the PR if that's the case.

@Aidanvii7
Copy link

also faced the same issue. I hope this gets merged soon! 🙏

@viktor-savchik-idf
Copy link

will it be merged?

@Alex009
Copy link
Member

Alex009 commented Feb 4, 2025

@viktor-savchik-idf yes i have plans to review and merge it. sorry for long waiting.

@Alex009 Alex009 changed the base branch from master to develop February 4, 2025 05:06
@Alex009 Alex009 added this to the 0.19.0 milestone Feb 7, 2025
@Alex009 Alex009 self-assigned this Feb 7, 2025
@Alex009 Alex009 merged commit 107cb10 into icerockdev:develop Feb 8, 2025
1 check passed
@pipetka7007
Copy link

Nice

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
5 participants