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

C::class.simpleName - possible problem with Proguard #227

Closed
aartikov opened this issue Oct 3, 2022 · 4 comments · Fixed by #228
Closed

C::class.simpleName - possible problem with Proguard #227

aartikov opened this issue Oct 3, 2022 · 4 comments · Fixed by #228
Labels
bug Something isn't working

Comments

@aartikov
Copy link

aartikov commented Oct 3, 2022

Hi, Arkadii.

As I can see the version 1.0.0-alpha-05 of Decompose uses simpleName for default keys of child stack and overlay.
It could cause a problem with Proguard when originally different class names are converted to the same name.
The bug will be catched only on a release (minified) build, that is unpleasant.

What do you think about replacing C::class.simpleName with C::class.qualifiedName? A qualifiedName is guaranteed to be unique.

@arkivanov
Copy link
Owner

arkivanov commented Oct 3, 2022

Thanks for raising! Unfortunately qualifiedName is not available in K/JS. But having the behavior potentially different on different kinds of builds is also not good. I would rather revert the change. Or maybe use KClass.hashCode in JS instead of qualifiedName. I will think about it and fix soon.

@arkivanov arkivanov added the bug Something isn't working label Oct 3, 2022
@aartikov
Copy link
Author

aartikov commented Oct 3, 2022

@arkivanov
Oh, didn't know that fact about Kotlin/JS.

I feel ok that I need to specify keys explicitly when there are multiple child stacks in a component. It is a quite rare case.
I would prefer safety rather than a little bit of boilerplate code.

What about KClass.hashCode, is it guaranteed to be unique? It is quite easy to find two Strings with the same hash code, for example "Aa".hashCode() == "BB".hashCode(). Is it better for KClass?

@arkivanov
Copy link
Owner

I think hashCode for a class is also not a good idea. It's not guaranteed to be stable, so e.g. after process death on Android, classes may have different hash codes. On JS - it's just simpleName.hashCode().

@arkivanov
Copy link
Owner

Version 1.0.0-alpha-06 is released, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants