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

refactor: use new stream feature #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

XadillaX
Copy link

This is a big refactor.

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全 stream 模式写还是得再改改


var ps = parseStream().pause();
stream.pipe(ps);
if (this._isAllStreamSizeKnown) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是否只需要 knownStreamSize 判断即可?


this._streams.push([leading, ps]);
if (options) {
for (const k in options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接assign 即可,不需要考虑老的代码

});

stream.on('data', data => {
this.push(data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这样不对的,formstream 一旦开始读,下游消费很慢,然后导致子 stream 数据全部被塞到内存里面了。这里应该也是得走 readable 模式,主动去读

@fengmk2
Copy link
Member

fengmk2 commented Sep 18, 2018

还需要管理传入来的 stream 异常情况

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.

2 participants