search

pinojs / pino-abstract-transport

Write Pino transports easily
MIT License
34 stars 13 forks source link

avoid duplexify for node >= 16.8 #29

Closed ronag closed 2 years ago

ronag commented 3 years ago

@mcollina

mcollina commented 3 years ago

Is it worth it? We are still bringing around the Duplexify dependency.

I think it's time I get back to packing the latest streams in readable-stream.

ronag commented 3 years ago

Is it worth it? We are still bringing around the Duplexify dependency.

I don't think the duplexify implementation is entirely correct...

I think it's time I get back to packing the latest streams in readable-stream.

Agreed. Based on Node 15, 16 or 17? I'd prefer if we could make readable-stream "latest and greatest" (i.e. master).

mcollina commented 3 years ago

master will be hard, I'd target v16.

ronag commented 3 years ago

A little off topic from this PR. But I think these commits from v17 should be in next version:

[0e841b45c2] - (SEMVER-MAJOR) stream: don't emit 'data' after 'error' or 'close' (Robert Nagy) #39639 [ef992f6de9] - (SEMVER-MAJOR) stream: do not emit end on readable error (Szymon Marczak) #39607 [09d8c0c8d2] - (SEMVER-MAJOR) stream: destroy readable on read error (Robert Nagy) #39342 [bb275ef2a4] - (SEMVER-MAJOR) stream: unify stream utils (Robert Nagy) #39294 [b2ae12d422] - (SEMVER-MAJOR) stream: throw on premature close in Readable[AsyncIterator] (Darshan Sen) #39117 [0738a2b7bd] - (SEMVER-MAJOR) stream: finished should error on errored stream (Robert Nagy) #39235 [954217adda] - (SEMVER-MAJOR) stream: error Duplex write/read if not writable/readable (Robert Nagy) #34385

They fix rather bad stuff and waiting with them for another year would be unfortunate.

ronag commented 3 years ago

Going back to PR. I still think this is a good idea. As it provides the most "correct" behavior possible and now that v16 is soon LTS I assume most users will start switching over to that.

mcollina commented 3 years ago

CI is failing due to lowered coverage, could you add a skip for those lines?