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

v1.0 #40

Open
wants to merge 49 commits into
base: master
Choose a base branch
from
Open

v1.0 #40

wants to merge 49 commits into from

Conversation

rgarcia
Copy link
Member

@rgarcia rgarcia commented Nov 11, 2013

Not ready to be merged

@jonahkagan
Copy link
Contributor

In the spirit of creating more controversy, I'd like to put in a vote for naming the understream object understream instead of _s. I think it's more explicit and avoids confusion with _. _s looks like it could be a typo.

@rgarcia
Copy link
Member Author

rgarcia commented Nov 12, 2013

Benefits of _s:

  • Invokes the same muscle-memory/areas of your brain that you associate with _, so it "feels right" typing _s.each, etc.
  • Easier to type

@azylman
Copy link
Contributor

azylman commented Nov 12, 2013

One thing that we lose when we have so little context like this (only the readable stream you want to apply some transformation to) is the ability to wrap all of the streams in a domain. Is there some way that we can maintain that feature here?

@rgarcia
Copy link
Member Author

rgarcia commented Nov 12, 2013

@azylman possibly chain() could take some configuration re: domains, but I'm also tempted to throw away the domain experiment. We can revisit when the error.coffee test is 1.0'd.

@jonahkagan
Copy link
Contributor

At this point we were only using domains to catch thrown errors, right? All emitted errors were being caught via the 'error' event. What thrown errors were we worried about? If it's just errors from user-supplied functions (e.g. fn in _s.each(stream, fn)), then can we just wrap those functions with a try/catch and then emit any caught errors?

@azylman
Copy link
Contributor

azylman commented Nov 12, 2013

@jonahkagan try/catch can't catch exceptions that are thrown asynchronously. So if a user-supplied function does some asynchronous action and throws an exception there, the only way to catch it is with domains.

@jonahkagan
Copy link
Contributor

Can we use a domain to wrap the function instead of a try/catch?

@azylman
Copy link
Contributor

azylman commented Nov 12, 2013

Maybe, but I'd worry that there's too much behavior around domains we still don't understand. For instance, that would require that each stream have its OWN domain. Will multiple domains running at the same time step on each other's toes? Ideally we could do something like:

d.create()
d.add stream
d.on 'error', (err) ->
  # Check if the error is the result of throwing so we don't emit errors that have already been emitted
  stream.emit 'error', err
d.exit()

But I don't know if that's going to intercept errors from other domains (which would be bad because we'd be re-emitting on the wrong stream), or if the exit is even necessary.

Rafael Garcia and others added 27 commits November 15, 2013 18:03
cc @understream-has-no-docs-haters
cc @jonahkagan not tested...not sure the best way to test it.
Could duplicate all existing tests and run them under aliases, but that
seems like overkill.
cc @jonahkagan @azylman I've been sneaking this feature in as I convert
things for 1.0, but all of the
mixins in the 1.0 branch default to adopting the objectMode of their
upstream, e.g. the stream returned by `_s.each(readable, fn)` will have
the same objectMode as `readable`. I've documented this in this commit,
let me know what you think.

I'd also like to add some sugar to the objectMode option so that you can
specify both the readable and writable side's objectMode:

```
_s.(readable).map(fn, { objectMode: {readable: false, writable: true} })
```

This would let you safely have streams that convert streams of binary/string
data into individual objects and vice versa.
cc @azylman I think this should fix travis. To be super user-friendly
I'm thinking we should also gate the features themselves, e.g. throw if
a user uses first/reduce/rest and their node version is bad.
@prime-time prime-time mentioned this pull request Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants