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

API Change Discussion: Switch to Single instead of Maybe #101

Closed
ZacSweers opened this issue Oct 3, 2017 · 3 comments · Fixed by #108
Closed

API Change Discussion: Switch to Single instead of Maybe #101

ZacSweers opened this issue Oct 3, 2017 · 3 comments · Fixed by #108
Assignees
Milestone

Comments

@ZacSweers
Copy link
Collaborator

This is something I think might be an inherent design flaw in AutoDispose currently. Maybe was used before under the idea that scopes could potentially not emit. However, it's left us in an awkward case where they could either emit or complete, which is different. Furthermore, it's actually not dangerous to depend on a single because in the event of termination of the source stream, it would just dispose the single it was waiting on.

This is a pretty significant API change. Fortunately I think it would be mitigated by the fact that most people use ScopeProvider and LifecycleScopeProvider + hopefully the test helpers.

@ZacSweers ZacSweers added this to the 1.0.0 milestone Oct 3, 2017
@ZacSweers ZacSweers self-assigned this Oct 3, 2017
@ZacSweers
Copy link
Collaborator Author

Devil's advocate: Scopes externally often do get represented as Maybe because they might not actually "end" per se. Consider a scope that is represented by a DB transaction, where you might want to show a loading indicator until some other stream emits. That stream might be represented by a Maybe as a database, and the flexibility to allow for it is helpful here. firstElement() also feels softer than coercing things via firstOrError().

@ZacSweers
Copy link
Collaborator Author

Alternatively - we could make it not a breaking change by adding more overloads for with() now, which is a lot simpler to maintain than it used to be. We can still just change implementation details to narrow down to a Single instead. I'm leaning toward this

@ZacSweers
Copy link
Collaborator Author

Note - ScopeProvider breaks in this change regardless :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant