Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

componentFromStream can potentially throw EmptyError: no elements in sequence #309

Merged
merged 4 commits into from Jan 29, 2017
Merged

componentFromStream can potentially throw EmptyError: no elements in sequence #309

merged 4 commits into from Jan 29, 2017

Conversation

avocadowastaken
Copy link
Contributor

@avocadowastaken avocadowastaken commented Jan 29, 2017

As from 0.22.0 componentFromStream will complete the stream during unmount and usage of first, last or single methods can potentially throw EmptyError.

See #220 and #303

@istarkov
Copy link
Contributor

are u sure https://github.com/acdlite/recompose/blob/master/src/packages/recompose/componentFromStream.js#L41
any example with real life error throwing?

@avocadowastaken
Copy link
Contributor Author

Had this problem in my project after update. It was cause with propsStream.filter(props => props.id > 0).first() like streams. I'll add test cases bit later.

@istarkov
Copy link
Contributor

Ill add a ref to this thread here https://github.com/acdlite/recompose/releases/tag/v0.22.0
dont think that documentation is the place for possible issues

@istarkov
Copy link
Contributor

Done, thank you.

@avocadowastaken
Copy link
Contributor Author

any example with real life error throwing?

Done

dont think that documentation is the place for possible issues

I thought it would be good for new users to know about this gotcha. And yes, for old users release description is better place.

@istarkov
Copy link
Contributor

May we move changes from documentation into first comment here and Ill merge the test only?

@istarkov
Copy link
Contributor

I think for new users it will be well described understandable rx error, isnt it?

@avocadowastaken avocadowastaken changed the title Add note about stream closing in componentFromStream() componentFromStream can potentially throw EmptyError: no elements in sequence Jan 29, 2017
@istarkov
Copy link
Contributor

Or may be change documentation as now its not obvious why such exception should occur

@avocadowastaken
Copy link
Contributor Author

May we move changes from documentation into first comment here and Ill merge the test only?

Done

Or may be change documentation as now its not obvious why such exception should occur

I'm not good at documenting things :).

@istarkov
Copy link
Contributor

A rare persons are good in documentation. My own projects some kind of documentation hell ;-)

@istarkov
Copy link
Contributor

Thank you!!!

@istarkov istarkov merged commit bed9c41 into acdlite:master Jan 29, 2017
@avocadowastaken avocadowastaken deleted the api-note branch January 29, 2017 17:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants