Comments (3)
Yes, I believe your assessment is correct. Internally we have a custom base class we use for our viewmodels and we mark that with @stable. However, while that should generally work, there is nothing stopping somebody from any public properties to their viewmodel and incorrectly using them from compose UI. For that reason I don't think it would be right to add the @stable annotation within this library.
I think updating the docs to suggest the best practice would be best. I think your second two options are both good. If people happen to have/want a custom base class they can put the annotation there, otherwise using compose_compiler_config.conf is a fine option.
Internally we have strong guidelines enforcing that no properties besides state flow are accessed off the viewmodel during composition to prevent mistakes and guarantee stability. We could possibly provide a util function to perform that validation on the viewmodel, but it would be up to users to set up a test to call that function. I'm thinking about something that takes the viewmodel as a parameter and uses reflection to enforce that all properties are private/protected (perhaps unless the return type has a @StableMarker annotation), and all public/internal functions only return Unit
.
The viewmodel already does some debug checks around proper state immutability, but I don't think it would be proper to add a stability check like this retroactively, so it would need to be opt in.
If you want to put up a PR to add those details to the docs, and/or work on a stability validation util, I'd be happy to review
from mavericks.
Thanks for sharing this, that's great! I hope others can reuse this and build on it. I'll close for now assuming there's no more to discuss
from mavericks.
Appreciate the info @elihart! For stability validation I ended up first making a PSI check that happened at compile time (hooked into Anvil) and then ended up converting that into a custom detekt rule. Hopefully these can be of use to someone...
Compile-time check:
/**
* Can be suppressed with `@Suppress("UnstableViewModel")`
*/
private fun ensureCanBeMarkedStable(clazz: Psi) {
val suppressed = clazz.annotations.any {
it.fqName == Suppress::class.fqName && it.arguments.any { arg -> arg.value<String>() == "UnstableViewModel" }
}
if (!suppressed) {
val publicProperties = clazz.properties
.filter { it.visibility() !in listOf(PRIVATE, PROTECTED) }
val functionsWithNonUnitReturn = clazz.functions
.filter { it.visibility() !in listOf(PRIVATE, PROTECTED) && it.returnTypeOrNull() != null }
if (publicProperties.isNotEmpty() || functionsWithNonUnitReturn.isNotEmpty()) {
val propErrors = publicProperties.joinToString(",") { "${it.name} should be private" }
val funcErrors = functionsWithNonUnitReturn.joinToString(",") { "${it.name} should return Unit" }
error("UnstableViewModel: ${clazz.fqName} would be considered an unstable type by the compose compiler: $propErrors $funcErrors")
}
}
}
Detekt Rule:
@RequiresTypeResolution
class UnstableMavericksViewModel : Rule() {
override val issue = Issue(
javaClass.simpleName,
Severity.Defect,
"MavericksViewModel classes should not have public/internal properties, and all public/internal functions should return Unit " +
"so that they are considered stable types by the compose compiler when used as params in compose functions",
Debt.TWENTY_MINS
)
override fun visitClass(klass: KtClass) {
if (klass.getSuperNames().contains("MavericksViewModel")) {
klass.getProperties()
.filter { it.isPublic || it.isInternal() }
.forEach { property ->
report(
CodeSmell(
issue = issue,
entity = Entity.from(klass),
message = "${property.name ?: "Properties"} should be private, " +
"ViewModel would be considered an unstable type by the compose compiler"
)
)
}
klass.declarations
.filterIsInstance<KtNamedFunction>()
.filter { it.isPublic || it.isInternal() }
.filter {
bindingContext[BindingContext.FUNCTION, it]?.returnType?.isUnit() != true
}
.forEach {function ->
report(
CodeSmell(
issue = issue,
entity = Entity.from(klass),
message = "${function.name ?: "Functions"} should return Unit, " +
"ViewModel would be considered an unstable type by the compose compiler"
)
)
}
}
}
}
They would both need additional logic to be added anywhere official, as they assume that all VMs are used in Composables which isn't necessarily true.
from mavericks.
Related Issues (20)
- compare with mvc,mvp,mvvm HOT 2
- The viewModel() method may be missing the postInvalidate() logic. HOT 2
- Hilt and ViewModel initialization with Navigation Component arguments HOT 6
- Exploration of the correct way to update state with List<Data> in state HOT 1
- Ensure that your state properties properly implement hashCode. HOT 7
- I wanted to use the latest version of Epoxy, Mavericks, Paging3 (Epoxy Paging3), but couldn't find the Demo. Is it possible to combine all three?
- java.lang.SecurityException: One of RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED should be specified
- java.lang.IllegalStateException HOT 1
- Value classes break State class no zero argument constructor usage HOT 1
- Discuss whenStarted function deprecation HOT 7
- I am using MavericksViewModel on Compose, I want to get the LifecycleOwner object in ViewModel how do I need to get this? HOT 3
- Crash when creating MavericksViewModel HOT 6
- Combining Mavericks with "custom" network response and error handling HOT 1
- Question - collectAsStateWithLifecycle HOT 4
- how to use maverik on activity ?
- Unable to resolve - hiltMavericksViewModelFactory HOT 3
- MavericksView invalidate not being called
- Now the time has come to support multiple platforms? HOT 1
- How is the logic of unequal old and new states
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mavericks.