setRawMode fails when running with non-TTY stdin

[eweilow]

Before the rest of the issue, I must say that Ink is amazing 👍

An issue I've encounted is that when Ink is running in a process where process.stdin.setRawMode is undefined (for instance in some CI environments), these lines will throw:
https://github.com/vadimdemedes/ink/blob/master/src/components/App.js#L73
https://github.com/vadimdemedes/ink/blob/master/src/components/App.js#L83

This behaviour is detailed by the Node docs on https://nodejs.org/api/tty.html#tty_readstream_setrawmode_mode

When Node.js detects that it is being run with a text terminal ("TTY") attached, process.stdin will, by default, be initialized as an instance of tty.ReadStream and both process.stdout and process.stderr will, by default be instances of tty.WriteStream. The preferred method of determining whether Node.js is being run within a TTY context is to check that the value of the process.stdout.isTTY property is true

After a search for setRawMode, it seems that this issue has resurfaced: #40

While the easy fix would be to wrap all those calls with a check for stdin.isTTY, that might lead to unexpected behaviour when an app actually expects raw mode to have been set!

[vadimdemedes]

While the easy fix would be to wrap all those calls with a check for stdin.isTTY, that might lead to unexpected behaviour when an app actually expects raw mode to have been set!
Not sure there's other solution, since it's impossible to listen for user input in non-tty env, isn't it? I think Ink should probably do nothing when setRawMode is called in a non-tty env. What do you think?

Before the rest of the issue, I must say that Ink is amazing 👍

Thank you!

[eweilow]

Not sure there's other solution, since it's impossible to listen for user input in non-tty env, isn't it? I think Ink should probably do nothing when setRawMode is called in a non-tty env. What do you think?

What might be good, since it's always nice to have a proper fallback, is to have some way for the app rendered by Ink to determine if setRawMode is available! I believe this might be useful for features like #147

In most cases, Ink would just ignore that it can't enable raw mode, but for some cases an app might want to entirely disable an input component. So just doing nothing when setRawMode is called in a non-tty environment works, but there could be additional functionality added to handle it nicely as well!

[mAAdhaTTah]

Yeah, I actually ran into this. I am trying to migrate some tests, where I would cp.spawn the cli and use the exposed stdin to write to the process and confirm the stdout matches my expectations as well as confirm the files written to the fs matched. The cli would error on the call to setRawMode and fail the tests.

I had originally attempted to use pty.js, but may have to give node-pty from the linked thread a shot. That said, it should still work w/o the call to setRawMode, so I kinda feel like the conditional would be a nice addition.

Also happy to PR this as well :).

[vadimdemedes]

What might be good, since it's always nice to have a proper fallback, is to have some way for the app rendered by Ink to determine if setRawMode is available!

@eweilow You don't need Ink for that, see how to check for a tty env: https://stackoverflow.com/questions/20585729/how-to-detect-if-nodes-process-stdout-is-being-piped.

[eweilow]

@vadimdemedes What if <StdinContext> exposed an additional isRawModeSupported bool?
It might lead to more 3rd party components adopting a nicer fallback if it was clear in the docs, even if it is currently as simple as checking if stdin.isTTY is true or not. Another option would be to simply add info on how to check for that in the docs of StdinContext 👍

[mAAdhaTTah]

I was able to solve my problem with node-pty, so I no longer need this. I will just note that components like ink-text-input might also need to add some handling for this if we don't solve it in Ink proper. My initial thought was to wrap setRawMode such that if it doesn't exist we just don't call it, which would keep downstream components from needing to add extra internal logic to check.

[eweilow]

I've started a PR (#172), but realized that it might be useful to throw a nice error by default rather than just ignoring that setRawMode cannot be called.

Otherwise this situation would end up with being able to not input anything (which would be confusing!) vadimdemedes/ink-text-input#25

[vadimdemedes]

Nice, thanks @eweilow! Let's continue discussion there.