[Bug] Win/MinGW - spinner() breaks confirm()/select(), but only for the second one about clack HOT 8 CLOSED

Just to follow up - can confirm that current main branch is all working as expected now.
Thanks everyone!

from clack.

Interesting. I wonder if something's not cleaned up correctly.

I don't have access to a Windows env, but I'll see if I can reproduce elsewhere or reason my way around potential issues.

Are the reproduction steps as minimal as possible?

from clack.

minimal as possible?

Replace examples/basic/index.ts with this:

import * as p from '@clack/prompts';
import { setTimeout } from 'node:timers/promises';
 
async function main() {
    console.clear();
 
    const confirmed1 = await p.confirm({
        message: `Here you can select\n\n   1\n`,
    });
    
    const confirmed2 = await p.confirm({
        message: `Here you can also select\n\n   2\n`,
    });
    
    const detecting = p.spinner();
    detecting.start();
    await setTimeout(2000);
    detecting.stop();
 
    const confirmed3 = await p.confirm({
        message: `FIRST after the spinner is OK\n\n   3\n`,
    });
    
    const confirmed4 = await p.confirm({
        message: `THIS is broken, needs one extra [enter] press, and then an [arrow-key]\n\n   4\n`,
    });
    
    const confirmed5 = await p.confirm({
        message: `and now this is ok again.\n\n   5\n`,
    });
 
}
 
main().catch(console.error);

Q1, Q2 and Q5 are purely for demonstration,
spinner & Q3 cause it, Q4 is the trouble

Ive cloned this repo and have poked around in it quite a bit with force lock/unlocking and things there like i found
all to no effect.

from clack.

Unable to reproduce, but I did find one bug in block(). No idea if it solves your problem or not. Could you try checking out the fix-block branch and see if it makes any difference?

from clack.

Hm... yes-and-no...
It does NOT fix it, but "its very close":

Going from that diff ive poked a bit more...

• fix-block makes no difference at all, the rl.close(); in the return does already take care of it

// branch: main
{ where: 'block()->return()-early', isTTY: true, isRaw: true }
{ where: 'block()->return()-post-off', isTTY: true, isRaw: true }
{ where: 'block()->return()-post-hide', isTTY: true, isRaw: true }
{ where: 'block()->return()-post-close', isTTY: true, isRaw: false }
{ where: 'main()->after-spinner/before-3', isTTY: true, isRaw: false }

• combined with this,
  the early if (input.isTTY) input.setRawMode(true); does not rly do anything,
  const rl = readline.createInterface... does already switch isRaw from false to true (at least for me here ;D)

{ where: 'main()->before-spinner', isTTY: true, isRaw: false }
{ where: 'block()-early', isTTY: true, isRaw: false }
{ where: 'block()-post-rl', isTTY: true, isRaw: true }
{ where: 'block()-post-rl-emit', isTTY: true, isRaw: true }
{ where: 'block()-post-set', isTTY: true, isRaw: true }

And I actually managed to "fix" it...

-> by DIRECTLY resetting raw mode, and not "waiting until the return()"
so inverting the existing line 20 to input.setRawMode(false);
Effectively resetting what readline.createInterface did 2 lines above before it "prints any output and waits"?

• and it still spins,
• prints animated dots,
• and ignores all but Ctrl+C as expected...

Sounds a bit spooky to me, but given that original code is only 2 weeks old it could be a thing i guess :D
Maybe @natemoo-re got smth to say? your commit 😋

diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts
@@ -17,7 +17,7 @@ export function block({
                tabSize: 1,
        });
        readline.emitKeypressEvents(input, rl);
-       if (input.isTTY) input.setRawMode(true);
+       if (input.isTTY) input.setRawMode(false);

        const clear = (data: Buffer, { name }: Key) => {
                const str = String(data);

{ where: 'main()->before-spinner', isTTY: true, isRaw: false }
{ where: 'block()-early', isTTY: true, isRaw: false }
{ where: 'block()-post-rl', isTTY: true, isRaw: true }
{ where: 'block()-post-rl-emit', isTTY: true, isRaw: true }
{ where: 'block()-post-set', isTTY: true, isRaw: false }

from clack.

I think we're encountering this in aicommits too:
Nutlope/aicommits#102

I went through the clack code and narrowed it down to this reproduction and fix:
nodejs/node#31762 (comment)

To apply it to this project, adding rl.terminal=false; above this line should fix it #80:

@wadabum
Would you mind seeing if the reproduction & fix works for you?

@ulken
I'm using a Mac with Virtual Box running Windows 11. It's a free setup if you want to try to reproduce (guide).

from clack.

Glad we got things rolling. Nice digging folks!

• fix-block makes no difference at all, the rl.close(); in the return does already take care of it

You're right. Then we should be able to remove all usages of setRawMode() in core as well?

For reference, I'm including some docs on the behavior of terminal.

From readline.createInterface()

terminal true if the input and output streams should be treated like a TTY, and have ANSI/VT100 escape codes written to it. Default: checking isTTY on the output stream upon instantiation.

From readline.emitKeypressEvents()

If the stream is a TTY, then it must be in raw mode.

This is automatically called by any readline instance on its input if the input is a terminal. Closing the readline instance does not stop the input from emitting 'keypress' events.

readline.emitKeypressEvents(process.stdin);
if (process.stdin.isTTY)
 process.stdin.setRawMode(true);

@ulken
I'm using a Mac with Virtual Box running Windows 11. It's a free setup if you want to try to reproduce (guide).

Thanks!

from clack.

@natemoo-re, I'm encountering this exact issue on v0.7.0. I used the same code mentioned above to reproduce. It breaks on Windows and works correctly on macOS. Could you please verify if you can replicate this issue as well?

Environment

• Node Version: v20.10.0
• Package Version: v0.7.0

import * as p from '@clack/prompts';
import { setTimeout } from 'node:timers/promises';

async function main() {
  console.clear();

  const confirmed1 = await p.confirm({
      message: `Here you can select\n\n   1\n`,
  });
  
  const confirmed2 = await p.confirm({
      message: `Here you can also select\n\n   2\n`,
  });
  
  const detecting = p.spinner();
  detecting.start();
  await setTimeout(2000);
  detecting.stop();

  const confirmed3 = await p.confirm({
      message: `FIRST after the spinner is OK\n\n   3\n`,
  });
  
  const confirmed4 = await p.confirm({
      message: `THIS is broken, needs one extra [enter] press, and then an [arrow-key]\n\n   4\n`,
  });
  
  const confirmed5 = await p.confirm({
      message: `and now this is ok again.\n\n   5\n`,
  });

}

main().catch(console.error);

from clack.