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

Remove @FlowPreview on (() ->T).asFlow() and (suspend () -> T).asFlow() #3097

Closed
hoc081098 opened this issue Dec 17, 2021 · 11 comments
Closed
Labels

Comments

@hoc081098
Copy link

hoc081098 commented Dec 17, 2021

I notice @FlowPreview has been removed on all extensions ###.asFlow() except () -> T and suspend () -> T. Can we remove it on them?

@qwwdfsad qwwdfsad added the flow label Dec 21, 2021
@qwwdfsad
Copy link
Contributor

We can gradually promote them to experimental.

Our original concern was top-level scope pollution, but it seems like it doesn't bother anyone.

Could you please elaborate on your usages of these extensions?

@martymiller
Copy link

martymiller commented Jul 5, 2022

We can gradually promote them to experimental.

Our original concern was top-level scope pollution, but it seems like it doesn't bother anyone.

What does this answer mean? Can we remove FlowPreview?

@hoc081098
Copy link
Author

Can v1.7 remove it?

@ryanholden8
Copy link

Yeah, can we remove it?

@martymiller
Copy link

martymiller commented Jul 27, 2022

Just wanted to share that I added this to my app level build.gradle.kts (inside of android { })

  tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile>().all {
    kotlinOptions.freeCompilerArgs += listOf(
      "-Xopt-in=kotlin.RequiresOptIn",
      "-Xopt-in=kotlin.OptIn",
      "-Xopt-in=kotlinx.coroutines.ExperimentalCoroutinesApi",
      "-Xopt-in=kotlinx.coroutines.ObsoleteCoroutinesApi",
      "-Xopt-in=kotlinx.coroutines.FlowPreview"
    )
  }

This allowed me to remove all the annotations in my code without any complaints or warnings from the IDE. When FlowPreview is eventually removed, I can just remove it from here.

@albertocavalcante
Copy link

It's not clear to me, without the compiler flags... can/should it be removed?

@fvasco
Copy link
Contributor

fvasco commented Dec 1, 2022

Can someone write some motivation to remove @FlowPreview?
What is the use case?

@JakeWharton
Copy link
Contributor

With annotations like @FlowPreview you actually need to make a justification in the reverse. What is the reason to continue preventing this API from being stable? Ideally every API annotated with it would be revisited at a regular cadence to ensure that each one needs to remain unstable for some valid reason.

@hoc081098
Copy link
Author

It is useful to convert a lambda to Flow, without flow { emit(...) }`.

suspend fun fetchApi(): Response

fetcherFlow
  .flatMapLatest { ::fetchApi.asFlow() }
  .collect { ... }

@mhernand40
Copy link

It is useful to convert a lambda to Flow, without flow { emit(...) }`.

suspend fun fetchApi(): Response

fetcherFlow
  .flatMapLatest { ::fetchApi.asFlow() }
  .collect { ... }

For this particular snippet, I'd argue that you can use mapLatest which accepts a suspend lambda rather than converting the suspend lambda into a one-shot Flow. 🤷‍♂️

@hoc081098
Copy link
Author

hoc081098 commented Dec 2, 2022

@mhernand40 of course, since it is a simple demo. When we use flatMapXXX, we usually transform the input flow to another flow...

suspend fun fetchApi(): Response

fetcherFlow
  .flatMapLatest {
    ::fetchApi.asFlow()
      .map { LCE.Success(it) }
      .onStart { emit(LCE.Loading) }
      .catch { emit(LCE.Error(it)) }
  }
  .collect { ... }

qwwdfsad added a commit that referenced this issue Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants