Call pull when needsMore is true #169
Conversation
| 1. Set `this.[[pulling]]` to **false**. | ||
| 1. Let _queueSize_ be GetTotalQueueSize(`this.[[queue]]`). | ||
| 1. Let _needsMore_ be ToBoolean(Invoke(`this.[[strategy]]`, `"needsMore"`, (_queueSize_))). | ||
| 1. Let _needsMore_ be the result of calling `this`.[[getNeedsMore]]. |
There was a problem hiding this comment.
Should be this.[[getNeedsMore]]() (the () being the important part)
Oh, thanks. I'll do so from next time. Now, I'll add reference implementation changes to this patch. |
|
|
Rewrote to
but looks too verbose. Maybe I can just write getNeedsMore as an abstract operation (#161). |
|
You can just use
Heh, see, this is bad, because [[getNeedsMore]] should not really be a function, so it does not/should not have a [[Call]] internal method. This is exactly the kind of confusion that #161 is meant to avoid. I wish I had just done that from the start. |
|
Done and added reference-implementation files. |
| @@ -272,6 +283,15 @@ export default class ReadableStream { | |||
| if (this._pulling === true) { | |||
| return; | |||
| } | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
You don't want to try/catch here; ReturnIfAbrupt just means letting the error bubble. Will fix while merging.
|
Pushed with minor fixes to https://github.com/whatwg/streams/tree/pull-needs-more but as per https://github.com/whatwg/streams/pull/169/files#r16258931 the new need to add strategies everywhere is worrying. I don't understand why these changes should cause the stream piping to stall with a zero high-water-mark. |
tyoshino
force-pushed
the
callpullonneedsmore
branch
from
0e688b7 to
93ce989
Compare
|
Thanks. Please see the inlined comment about the issue with Here's a sample of fix for |
|
Note: This pull req is for #153 |
I'm creating this pull request so I can comment on it; apparently you can't comment on things without them being pull requests :P