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

Rewrite resource loading mechanism #524

Closed
jrodbx opened this issue Jul 30, 2022 · 2 comments
Closed

Rewrite resource loading mechanism #524

jrodbx opened this issue Jul 30, 2022 · 2 comments
Assignees
Milestone

Comments

@jrodbx
Copy link
Collaborator

jrodbx commented Jul 30, 2022

During our hack week, we used com.android.ide.common.resources.deprecated.ResourceRepository mainly due its simplicity -- pass it a file path to a resource source set directory, and it will parse and load the contents for later retrieval and resolution for free. However, this left a bit of tech debt.

First, this ResourceRepository definition has since been deleted from sdk-common and replaced with a different interface implemented in the Android Studio project. As a result, we can't bump some of LayoutLib's transitive dependencies (sdk-common, layoutlib-api).

Second, this also couples us slightly to AGP's merged resources dir, which contains all the transitive resources that drive Paparazzi. Following the new ResourceRepository approach would instead reference and parse the project resource files in /src/main/res (and variants), artifact resource files in aar/res, as well as the framework resources, provided by layoutlib, directly.

Since Paparazzi tests should also run on the CLI, the new ResourceRepository implementation cannot use a similar approach and use Intellij's PSI model to reference the resource source set directories. Instead, the implementation would likely have to rely on AGP bits to obtain them (heads up, Bazel folk, this would be another thing to implements) which is still better than coupling to AGP's intermediary output directories.

Fixing this should automatically also provide support for application modules (since we wouldn't be looking into build/intermediates/merged_res anymore, which is in a binary format for application modules) and fix the issue with HTML string resources since a different resource parsing path is used altogether.

I've been putting this off for a while, but it's about time to tackle this since we'll be rewriting/refactoring the core of Paparazzi on the march to 2.0.

@jrodbx jrodbx self-assigned this Jul 30, 2022
@geoff-powell geoff-powell self-assigned this Aug 26, 2022
@jrodbx jrodbx added this to the 1.2 milestone Sep 20, 2022
@jrodbx jrodbx modified the milestones: 1.2, 1.3 Jan 4, 2023
@jrodbx jrodbx modified the milestones: 1.3, 1.3.1 May 31, 2023
@jrodbx jrodbx assigned jrodbx and unassigned jrodbx Jun 10, 2023
@jrodbx
Copy link
Collaborator Author

jrodbx commented Jul 8, 2023

This should now be fixed, and will be promoted in the upcoming (1.3.1) release.

@jrodbx jrodbx closed this as completed Jul 8, 2023
@jisungbin
Copy link

Thanks for the awesome work! 🫶🫶 (@jrodbx @kevinzheng-ap and other Paparazzi contributors!)

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

No branches or pull requests

4 participants