vadimdemedes / ink

🌈 React for interactive command-line apps
https://term.ink
MIT License
26.54k stars 598 forks source link

Static + exit results in broken/mismatched output #397

Open milesj opened 3 years ago

milesj commented 3 years ago

I'm working on a command that does some validation and outputs it nicely to the screen via Static, since it only needs to render once. If there are any errors, I want to hard fail with a non-zero exit code, so I'm using exit. Here's some stripped down example code:

const React = require('react');
const { Box, Text, Static, useApp } = require('ink');

const items = [{ key: 'foo' }, { key: 'bar' }, { key: 'baz' }];

function ThrowError() {
  const { exit } = useApp();

  React.useEffect(() => {
    exit(new Error('Exit'));
  }, [exit]);

  return (
    <Box>
      <Static items={items}>{({ key }) => <Text key={key}>{key}</Text>}</Static>
      <Text>Content</Text>
    </Box>
  );
}

I would expect the above to output the static items, then cause an exit. However, this is what I receive.

foo
bar
baz
foo
bar
baz
Content

2 sets of static output, no Exit thrown error, AND it doesn't set an exit code! It also seems to be rendering Content, which it should not be since it's not static? HOWEVER, if I comment out the Static component and re-run, I now receive this (using my custom error renderer).

Content

 ERROR 

 Exit

 STACK TRACE 

    at /boost/packages/cli/examples/commands/ErrorCompCommand.js:13:10
    at commitHookEffectList (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:12025:26)
    at commitPassiveHookEffects (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:12059:11)
    at Object.invokeGuardedCallbackImpl (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:11563:10)
    at invokeGuardedCallback (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:11740:31)
    at flushPassiveEffectsImpl (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15390:7)
    at unstable_runWithPriority (/boost/node_modules/scheduler/cjs/scheduler.development.js:697:12)
    at runWithPriority (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:1881:10)
    at flushPassiveEffects (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15359:12)
    at /boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15238:11

Now I see the Exit error and Content (which I assume to be the last render before unmount). It seems like Static somehow swallows the output/control of exit, and I'm not sure how to work around it. If everything was working correctly, I would expect the following output.

foo
bar
baz
Content

 ERROR 

 Exit

 STACK TRACE 

    at /boost/packages/cli/examples/commands/ErrorCompCommand.js:13:10
    at commitHookEffectList (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:12025:26)
    at commitPassiveHookEffects (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:12059:11)
    at Object.invokeGuardedCallbackImpl (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:11563:10)
    at invokeGuardedCallback (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:11740:31)
    at flushPassiveEffectsImpl (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15390:7)
    at unstable_runWithPriority (/boost/node_modules/scheduler/cjs/scheduler.development.js:697:12)
    at runWithPriority (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:1881:10)
    at flushPassiveEffects (/boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15359:12)
    at /boost/node_modules/react-reconciler/cjs/react-reconciler.development.js:15238:11

I'm experimenting with errors here https://github.com/milesj/boost/pull/118 and stumbled upon this bug. Or maybe not a bug, you never know.

LaurensRietveld commented 2 years ago

I encountered the same issue. A workaround I'm using is wrapping the exit in a setTimeout:

React.useEffect(() => {
    setTimeout(() => exit(new Error('Exit')), 0);
  }, [exit]);
vadimdemedes commented 2 years ago

Thanks for reporting this!

Before I explain why is this happening, some required context about how Static component works. Static updates its output whenever React updates the component tree. So if for some reason, Ink attempts to render the output, but React didn't trigger an update, Ink renders the same Static content twice. It's still the same content, because there was no update in React tree, so Static didn't clear the last children.

This is exactly what's happening when you exit an App using exit method immediately after app is mounted, without any updates. When you call an exit function, it propagates to an internal unmount method, which calls onRender. This onRender method is what Ink uses to get the React component three and render it to a string → https://github.com/vadimdemedes/ink/blob/master/src/ink.tsx#L115-L172.

So the fix seems to be as simple as not calling onRender when app is exiting, but I'm guessing it's there for some reason and I made a mistake of not writing any comment there. While I'm looking at git history, thought I'd post these details to give you an update.

vadimdemedes commented 2 years ago

There doesn't seem to be anything in the git history that's pointing as to why there's an onRender call after unmount. Looks like it's been there since the beginning. I don't see any reason why it should be there now, so perhaps I should release a major version update as a precaution.