ADFA-4182: Project-resource rendering + full @Preview support#1370
Conversation
- Render with the project APK's resources/theme/config (images, uiMode, locale, fontScale) - All @Preview attrs, multi-preview, and @PreviewParameter (one card per value) - Contain composition errors inline; stabilize classloader/daemon/asset lifecycle - Feed editor edits into a live preview
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 Walkthrough
WalkthroughRefactors Compose preview parsing, state, loading, rendering, and resource wiring to support multiple preview instances with parameter-value expansion, APK-backed resource contexts, debounced source parsing, coordinated async dex/class loading, and per-instance rendering orchestration. ChangesCompose Preview Multi-Preview and Parameter Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewActivity.kt (1)
402-418: ⚡ Quick winNarrow the exception type from
ThrowabletoException.Catching
ThrowableincludesErrorsubclasses (OutOfMemoryError,StackOverflowError) which generally should not be caught. For reflection operations,ExceptionorReflectiveOperationExceptionprovides sufficient coverage.Based on learnings: "prefer narrow exception handling that catches only the specific exception type reported in crashes (such as IllegalArgumentException) instead of a broad catch-all."
♻️ Proposed fix
- } catch (e: Throwable) { + } catch (e: Exception) { LOG.error("Failed to resolve `@PreviewParameter` values from {}", providerFqn, e) emptyList() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewActivity.kt` around lines 402 - 418, The method resolveParameterValues currently catches Throwable which also captures Errors; change the catch to a narrower type (e.g., catch(Exception) or catch(ReflectiveOperationException)) so only reflection/checked exceptions are handled. Locate resolveParameterValues and replace the "catch (e: Throwable)" clause with "catch (e: Exception)" or the more specific "catch (e: ReflectiveOperationException)" and keep the existing logging call (LOG.error("Failed to resolve `@PreviewParameter` values from {}", providerFqn, e)) and return emptyList() as before; ensure this covers exceptions thrown by providerClass.getDeclaredConstructor(), newInstance(), and providerClass.getMethod("getValues").invoke(...).compose-preview/src/main/java/com/itsaky/androidide/compose/preview/data/source/ProjectContextSource.kt (1)
104-110: 💤 Low valueConsider handling absolute
outputFilepaths explicitly.The
outputFilevalue from the JSON listing may be an absolute path. WhileFile(listing.parentFile, outputFile)happens to work on POSIX systems (the absolute child path overrides the parent), this is a subtle and non-obvious behavior. Consider checkingFile(outputFile).isAbsolutefirst for clarity.♻️ Optional improvement
val outputFile = elements.optJSONObject(i)?.optString("outputFile").orEmpty() if (outputFile.endsWith(".apk")) { - val candidate = File(listing.parentFile, outputFile) + val raw = File(outputFile) + val candidate = if (raw.isAbsolute) raw else File(listing.parentFile, outputFile) if (candidate.exists()) { return candidate }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/data/source/ProjectContextSource.kt` around lines 104 - 110, The code currently constructs candidate = File(listing.parentFile, outputFile) without handling absolute paths; update the logic in ProjectContextSource.kt around the outputFile handling so that you first check File(outputFile).isAbsolute and, if true, use File(outputFile) as the candidate, otherwise use File(listing.parentFile, outputFile); then perform the existing candidate.exists() check and return if present. This keeps the existing variable names (outputFile, listing, candidate) and behavior but makes absolute paths explicit and clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeClassLoader.kt`:
- Around line 33-35: The current comparison uses unordered sets so reorderings
of projectDexFiles are treated as unchanged; update the equality check to
compare ordered lists instead of sets so DEX precedence changes trigger
release(): replace the set-based comparison of
existingFiles.mapTo(mutableSetOf()) { it.absolutePath } ==
projectDexFiles.mapTo(mutableSetOf()) { it.absolutePath } with an ordered
comparison (e.g., compare lists of absolutePath in the same order) in the
ComposeClassLoader code where existingFiles and projectDexFiles are used,
ensuring that when the ordered lists differ you call release() to reload
DexClassLoader.
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ProjectResourceContextFactory.kt`:
- Around line 72-75: The companion-object reflection to obtain
AssetManager.addAssetPath (ADD_ASSET_PATH) runs at class init and can throw on
non-SDK restricted devices; move the reflective lookup out of eager
initialization and guard it with a try/catch (or make it lazy) so failures don’t
cause ExceptionInInitializerError—e.g., replace the direct getMethod(...) in the
companion with a nullable cached field initialized inside a try/catch or a lazy
block that returns null on failure, and update assetsFor/its invoke(...) usage
to check the nullable ADD_ASSET_PATH before attempting to invoke it and fall
back to appContext.createConfigurationContext(...) as already intended.
---
Nitpick comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewActivity.kt`:
- Around line 402-418: The method resolveParameterValues currently catches
Throwable which also captures Errors; change the catch to a narrower type (e.g.,
catch(Exception) or catch(ReflectiveOperationException)) so only
reflection/checked exceptions are handled. Locate resolveParameterValues and
replace the "catch (e: Throwable)" clause with "catch (e: Exception)" or the
more specific "catch (e: ReflectiveOperationException)" and keep the existing
logging call (LOG.error("Failed to resolve `@PreviewParameter` values from {}",
providerFqn, e)) and return emptyList() as before; ensure this covers exceptions
thrown by providerClass.getDeclaredConstructor(), newInstance(), and
providerClass.getMethod("getValues").invoke(...).
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/data/source/ProjectContextSource.kt`:
- Around line 104-110: The code currently constructs candidate =
File(listing.parentFile, outputFile) without handling absolute paths; update the
logic in ProjectContextSource.kt around the outputFile handling so that you
first check File(outputFile).isAbsolute and, if true, use File(outputFile) as
the candidate, otherwise use File(listing.parentFile, outputFile); then perform
the existing candidate.exists() check and return if present. This keeps the
existing variable names (outputFile, listing, candidate) and behavior but makes
absolute paths explicit and clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b76bb3d5-5118-4d71-b087-97a8a4ca8bde
📒 Files selected for processing (11)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewActivity.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewFragment.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/ComposePreviewViewModel.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/compiler/CompilerDaemon.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/data/source/ProjectContextSource.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/domain/PreviewSourceParser.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeClassLoader.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ProjectResourceContextFactory.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/ui/BoundedComposeView.kt
- ComposeClassLoader: compare ordered DEX path lists (preserve classpath precedence) - ProjectResourceContextFactory: lazy/guarded addAssetPath lookup to avoid class-init crash on restricted devices
Screen_recording_20260604_235821.webm