Is <this change> intended to solve internal issues...
# compose
z
Is this change intended to solve internal issues, or external ones? Exhaustive when breaking when enums are changed is a feature of kotlin, I would think if consumers are writing exhaustive whens on these things then they’d want them to break if the API changes.
s
@Adam Powell
External.
a
yes, which is why we're making a deliberate distinction between sets of constants that may grow in the future and ones that won't
z
so the ones being converted to inline classes will or won’t grow in the future?
s
Have a possibility of "will", or we dont know that it "wont"
z
Ok, so you’re trying to avoid the case where someone upgrades the compose version in their codebase and suddenly the have compiler errors because a
when
somewhere isn’t handling the new enums, right?
s
I missed that specific meeting :) but yes
z
That seems to imply that you’re expecting that developer’s intent was not to exhaustively cover all the cases. However, if there intent was to cover all the cases, then there’s no longer any way to do that with this change. But without this change, it’s still possible to have a non-exhaustive
when
by adding an
else
branch. I guess i’m confused because this sort of thing seems like it’s intentionally subverting Kotlin language features, features which developers might actually want to use, and which they might be surprised to not have access to.
👍 1
But I also assume yall have a good reason for doing it, which I’m wondering what that is.
(thanks for humoring me so far 😛 )
a
These changes are far from all of the enum classes in compose; the ones where we're making an API promise to be exhaustive and not break downstream code are remaining enum classes
and it's not just compiler errors when you update; let's say you're using accompanist 1.2.0 which depends on compose-ui:1.1.0. compose-ui:1.2.0 is released, which adds new enum class values somewhere. You update to it without incident. You then start seeing crash reports because a previously exhaustive
when
somewhere in accompanist is no longer exhaustive and the when expression can't be evaluated.
We don't want to create situations where apps are held back from updating compose until all of their other compose-using dependencies update first, which makes enum class the wrong tool for many of these APIs.
z
Ah, that makes sense. I figured transitive deps had something to do with it.