Don't resolve plugin prompt promise until modal has closed, to avoid a race condition (#3970)

Co-authored-by: James Gatz <jamesgatzos@gmail.com>

packages/insomnia-app/app/plugins/context/__tests__/app.test.ts

@@ -80,6 +80,7 @@ describe('app.prompt()', () => {
           title: 'Title',
           onComplete: expect.any(Function),
           onCancel: expect.any(Function),
+          onHide: expect.any(Function),
         },
       ],
       [
@@ -88,6 +89,7 @@ describe('app.prompt()', () => {
           label: 'Label',
           onComplete: expect.any(Function),
           onCancel: expect.any(Function),
+          onHide: expect.any(Function),
         },
       ],
     ]);

packages/insomnia-app/app/plugins/context/app.tsx

@@ -69,7 +69,11 @@ export function init(renderPurpose: RenderPurpose = RENDER_PURPOSE_GENERAL): {
           return Promise.resolve(options.defaultValue || '');
         }
 
-        return new Promise((resolve, reject) => {
+        // This custom promise converts the prompt modal from being callback-based to reject when the modal is cancelled and resolve when the modal is submitted and hidden
+        return new Promise<string>((resolve, reject) => {
+          let shouldResolve = false;
+          let resolveWith: string | null = null;
+
           showPrompt({
             title,
             ...(options || ({} as Record<string, any>)),
@@ -79,7 +83,15 @@ export function init(renderPurpose: RenderPurpose = RENDER_PURPOSE_GENERAL): {
             },
 
             onComplete(value: string) {
-              resolve(value);
+              shouldResolve = true;
+              resolveWith = value;
+            },
+
+            // don't resolve the overall promise until the modal has hidden after clicking submit
+            onHide() {
+              if (shouldResolve && resolveWith !== null) {
+                resolve(resolveWith);
+              }
             },
           });
         });

packages/insomnia-app/app/ui/components/modals/prompt-modal.tsx

@@ -25,6 +25,7 @@ interface State {
   cancelable?: boolean | null;
   onComplete?: (arg0: string) => Promise<void> | void;
   onCancel?: (() => void) | null;
+  onHide?: () => void,
   onDeleteHint?: ((arg0: string) => void) | null;
   currentValue: string;
   loading: boolean;
@@ -44,6 +45,7 @@ export interface PromptModalOptions {
   label?: string;
   hints?: string[];
   onComplete?: (arg0: string) => Promise<void> | void;
+  onHide?: () => void,
   onDeleteHint?: (arg0: string) => void;
   onCancel?: () => void;
 }
@@ -69,6 +71,7 @@ class PromptModal extends PureComponent<{}, State> {
     onComplete: undefined,
     onCancel: undefined,
     onDeleteHint: undefined,
+    onHide: undefined,
     currentValue: '',
     loading: false,
   }
@@ -162,6 +165,7 @@ class PromptModal extends PureComponent<{}, State> {
     onCancel,
     validate,
     onDeleteHint,
+    onHide,
   }: PromptModalOptions) {
     this.setState({
       currentValue: '',
@@ -181,6 +185,7 @@ class PromptModal extends PureComponent<{}, State> {
       validate,
       hints: hints || [],
       loading: false,
+      onHide,
     });
     this.modal?.show();
 
@@ -276,7 +281,7 @@ class PromptModal extends PureComponent<{}, State> {
       'form-control--outlined': inputType !== 'checkbox',
     });
     return (
-      <Modal ref={this._setModalRef} noEscape={!cancelable || loading} onCancel={this._handleCancel}>
+      <Modal ref={this._setModalRef} noEscape={!cancelable || loading} onCancel={this._handleCancel} onHide={this.state.onHide}>
         <ModalHeader>{title}</ModalHeader>
         <ModalBody className="wide">
           <form onSubmit={this._handleSubmit} className="wide pad">

packages/insomnia-app/app/ui/components/modals/the-plan-for-modals.md

@@ -5,13 +5,14 @@
 1. We're trying to move the entire codebase to hooks.  With the possible exception of one component (ErrorBoundary) there should be no component in the codebase that must be a class.
 1. Right now, all modals are classes.  One core reason for this is that `showModal` in `app/ui/components/modals/index.ts` directly calls `show` on the modal instance of a registered modal.
 
-## Where we're going
+## Where we're going (TBC, but this is an initial plan)
 
 1. Modal lifecycle state (hiding, showing, loading) will be managed with redux actions.  This will eliminate the current blocker for moving to hooks.
 1. All modals will be hooks.
     1. Modals will thereby do their part in the larger mission of dropping the class-autobind-decorator dependency.
 1. Modals will use portals.
 1. Modals will not have to be singletons, making them more easily composable in the app's flows.
+    1. For an example of why singleton modals cause issues, see the regression that was resolved in [#3970](https://github.com/Kong/insomnia/pull/3970)
 1. Modals will not have to be loaded into the react tree at all times.
 
 ## How we're going to get there