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

readline module and streams #16178

Closed
Ginden opened this issue Oct 13, 2017 · 7 comments
Closed

readline module and streams #16178

Ginden opened this issue Oct 13, 2017 · 7 comments
Labels
feature request Issues that request new features to be added to Node.js. readline Issues and PRs related to the built-in readline module.

Comments

@Ginden
Copy link

Ginden commented Oct 13, 2017

Currently, readline module interface is rather special. Interface class is EventEmitter acting as stream, but not conforming to Stream interface. It's possible to redirect output to stream, but it's significantly less convenient than just .pipe.

Then I propose one of the following:

  • add class readline.Stream subclassing Transform.
    • readline.Interface can be backed by this stream, then no code duplication occurs. Though, I'm not sure about viability of this approach, as Interface contains lots of code related to TTY handling.
  • make readline.Interface instances subclasses of Transform stream

Compare:

foo = getReadableStream();
bar = transformStream();
foo
   .pipe(readline.Stream())
   .pipe(bar)
   .pipe(process.stdout)

vs.

foo = getReadableStream();
bar = transformStream();
readline.createInterface({
   input: foo,
   output: bar
}); // We are using factory function for side effects.
bar.pipe(process.stdout);

User-land packages providing this functionality:

  • byline (423k downloads per month)
@joyeecheung joyeecheung added readline Issues and PRs related to the built-in readline module. feature request Issues that request new features to be added to Node.js. labels Oct 13, 2017
@Fishrock123
Copy link
Contributor

We could do this but I would like to point out that it specifically does not pretend to act like a stream, nor do the docs state that it is any type of stream, it merely acts on a stream and can provide an output stream. That makes it transform-like but does not inherently make it a "transform stream".

@Ginden
Copy link
Author

Ginden commented Oct 14, 2017

We could do this

I'm willing to make a pull request if there is green light for this.

I believe that Interface instance is conceptually a stream. It reads data in chunks and emits other chunks based on source asynchronously. Making it implement Stream semantics is a change in interface, not in principle of working.

@Fishrock123
Copy link
Contributor

Fair enough. I think I'd prefer if the first option was attempted first, not sure how backwards-compat and API surface overlap would work out for the second option.

@bengl
Copy link
Member

bengl commented Oct 25, 2017

@Ginden You should go ahead and do a PR for this (the first option). FWIW, I'd really like to see this happen.

@clakech
Copy link
Contributor

clakech commented Feb 23, 2018

Hello, I would like this in a future nodejs version please ;)

@refack
Copy link
Contributor

refack commented Nov 11, 2018

@refack refack closed this as completed Nov 11, 2018
@zavan
Copy link

zavan commented Feb 9, 2024

I see that project is closed/archived. Is this being tracked elsewhere?

Edit: Nevermind, I figured out you can do this with pipeline:

import { pipeline } from "node:stream";
import { createInterface } from "node:readline";

pipeline(
  process.stdin,
  (input) => createInterface({ input }),
  process.stdout,
  (err) => {
    if (err) {
      console.error('Pipeline failed.', err);
    } else {
      console.log('Pipeline succeeded.');
    }
  }
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. readline Issues and PRs related to the built-in readline module.
Projects
None yet
Development

No branches or pull requests

7 participants