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

yield* #3

Merged
merged 1 commit into from
Nov 10, 2013
Merged

yield* #3

merged 1 commit into from
Nov 10, 2013

Conversation

jonathanong
Copy link
Member

use yield* internally. downstream must be a generator if it exists.

@jonathanong
Copy link
Member Author

if we do this, then we have to use the function* (next) {} signature. it's no longer backwards compatible with function (next) { return function* () {} }

@tj
Copy link
Member

tj commented Nov 10, 2013

hmm im seeing super minimal gains, ~100rps, not even worth breaking the old compat, a single console.log() does more damage than that

@jonathanong
Copy link
Member Author

yeah, it's not a big deal, but it's an easy win and it helps people learn more about generators. after we change all our examples/middleware/tests to the new signature i don't think there's a reason to support the old signature.

hmmm. how about just making noop a generator for now? we can do the yield* next stuff later.

the end goal is to be able to do yield* next without worrying that next isn't a generator (otherwise it'll throw) and i like forcing middleware to being only a generator function.

@tj
Copy link
Member

tj commented Nov 10, 2013

yeah I'd rather have yield next; than yield *next; any day

@jonathanong
Copy link
Member Author

well it's interchangeable. as long as next is a generator it doesn't matter. you can decide which signature you want to use. i just don't want to worry about yield* next throwing.

@jonathanong
Copy link
Member Author

i'm just gonna change noop to a generator function for now.

jonathanong added a commit that referenced this pull request Nov 10, 2013
@jonathanong jonathanong merged commit 767da19 into master Nov 10, 2013
@jonathanong jonathanong deleted the yield-delegate branch November 10, 2013 23:58
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.

None yet

2 participants