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

Don't typealias Month (and others) #96

Open
fluidsonic opened this issue Jan 31, 2021 · 5 comments · May be fixed by #378
Open

Don't typealias Month (and others) #96

fluidsonic opened this issue Jan 31, 2021 · 5 comments · May be fixed by #378
Assignees
Labels
breaking change This could break existing code
Milestone

Comments

@fluidsonic
Copy link

I suggest to not typealias anything in a multiplatform date/time library.

  • It mixes platform-provided API into the library's API which causes confusion and is platform-dependent.
  • It doesn't allow for adding a companion and thus "static" functionality - not even through extensions.

For example:
I had code in the back-end that used Month.of(someInt). The code was relatively simple and there were no JVM-specific imports. So I've moved it to a shared multiplatform dependency and was surprised that Month.of() was no longer available. Month.<autocomplete> didn't provide me with any useful alternative.

After Cmd-clicking on Month I've learned that it is merely a typealias on JVM. It took a while to figure out that there's an additional Month(Int) function in kotlinx-datetime that I should've used instead. Scenarios like that can easily lead to confusion and inconsistency.

@september669
Copy link

Another example for Android

import android.content.Context
import kotlinx.datetime.DayOfWeek

fun DayOfWeek.toString(context: Context): String{
    return when(this){
        java.time.DayOfWeek.MONDAY -> TODO() <---- Field requires API Level 26
        java.time.DayOfWeek.TUESDAY -> TODO()
        java.time.DayOfWeek.WEDNESDAY -> TODO()
        java.time.DayOfWeek.THURSDAY -> TODO()
        java.time.DayOfWeek.FRIDAY -> TODO()
        java.time.DayOfWeek.SATURDAY -> TODO()
        java.time.DayOfWeek.SUNDAY -> TODO()
    }
}

@JakeWharton
Copy link

@september669 the whole library requires coreLibraryDesugaring to be enabled enabling the use of java.time on previous API levels.

@september669
Copy link

september669 commented Mar 17, 2021

@september669 the whole library requires coreLibraryDesugaring to be enabled enabling the use of java.time on previous API levels.

Thanks a lot!

@PaulWoitaschek
Copy link

PaulWoitaschek commented Apr 15, 2021

I actually like the typealiasing as when we further process the data in the android project, we'd need need to create a wrapper that does the mapping.
We actually use a expect typealias LocalDate function to expose public properties coming from our library surface and actual implement it as javas LocalDate on android and NSDateComponents for ios.

The argument with the factories will go away when the namespace feature arrives.

@fluidsonic
Copy link
Author

fluidsonic commented May 25, 2021

With Kotlin 1.5 it became super annoying and really needs to be changed.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This could break existing code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@JakeWharton @fluidsonic @PaulWoitaschek @september669 @dkhalanskyjb and others