From 454975c01b810913480473adec1b985e42bf4ea0 Mon Sep 17 00:00:00 2001
From: Andy Brown <asbrown002@gmail.com>
Date: Thu, 30 Apr 2020 17:07:52 -0700
Subject: [PATCH] fix: do not over fetch the project (#2842)

* get rid of dialog router, reduce project fetches

* make definitions required on FieldProps

* show loading spinner when project is fetching

Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
---
 .../client/src/pages/design/index.tsx         | 36 +++++++--------
 Composer/packages/client/src/router.tsx       | 44 +++++++++----------
 Composer/packages/client/src/styles.ts        |  2 +
 .../src/components/SchemaField.tsx            | 15 ++++---
 .../src/components/fields/EditableField.tsx   |  2 +-
 .../src/utils/resolveBaseSchema.ts            |  5 ++-
 .../adaptive-form/src/utils/resolveRef.ts     |  2 +-
 .../extensions/extension/src/types/form.ts    |  2 +-
 .../prompts/src/PromptField/index.tsx         |  1 +
 .../src/BeginSkillDialogField.tsx             |  4 +-
 10 files changed, 59 insertions(+), 54 deletions(-)

diff --git a/Composer/packages/client/src/pages/design/index.tsx b/Composer/packages/client/src/pages/design/index.tsx
index bac0231326..58ae43d367 100644
--- a/Composer/packages/client/src/pages/design/index.tsx
+++ b/Composer/packages/client/src/pages/design/index.tsx
@@ -7,7 +7,7 @@ import React, { Suspense, useContext, useEffect, useMemo, useState, useRef } fro
 import { Breadcrumb, IBreadcrumbItem } from 'office-ui-fabric-react/lib/Breadcrumb';
 import { Icon } from 'office-ui-fabric-react/lib/Icon';
 import formatMessage from 'format-message';
-import { globalHistory } from '@reach/router';
+import { globalHistory, RouteComponentProps } from '@reach/router';
 import get from 'lodash/get';
 import { PromptTab } from '@bfc/shared';
 import { DialogFactory, SDKKinds, DialogInfo } from '@bfc/shared';
@@ -88,7 +88,7 @@ const getTabFromFragment = () => {
   }
 };
 
-function DesignPage(props) {
+const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: string }>> = props => {
   const { state, actions } = useContext(StoreContext);
   const visualPanelRef: React.RefObject<HTMLDivElement> = useRef<HTMLDivElement>(null);
   const { dialogs, designPageLocation, breadcrumb, visualEditorSelection, projectId, schemas } = state;
@@ -102,7 +102,7 @@ function DesignPage(props) {
     clearUndoHistory,
     onboardingAddCoachMarkRef,
   } = actions;
-  const { location, match } = props;
+  const { location } = props;
   const { dialogId, selected } = designPageLocation;
   const [triggerModalVisible, setTriggerModalVisibility] = useState(false);
   const [dialogJsonVisible, setDialogJsonVisibility] = useState(false);
@@ -118,15 +118,15 @@ function DesignPage(props) {
     }
     const rootDialog = dialogs.find(({ isRoot }) => isRoot === true);
     if (!currentDialog && rootDialog) {
-      const { search } = location;
+      const { search } = location || {};
       navigateTo(`/bot/${projectId}/dialogs/${rootDialog.id}${search}`);
       return;
     }
   }, [dialogId, dialogs, location]);
 
   useEffect(() => {
-    if (match) {
-      const { dialogId, projectId } = match;
+    if (location && props.dialogId && props.projectId) {
+      const { dialogId, projectId } = props;
       const params = new URLSearchParams(location.search);
       setDesignPageLocation({
         dialogId: dialogId,
@@ -400,18 +400,16 @@ function DesignPage(props) {
           onDeleteTrigger={handleDeleteTrigger}
         />
         <div role="main" css={contentWrapper}>
-          {match && (
-            <ToolBar
-              toolbarItems={toolbarItems}
-              actions={actions}
-              projectId={projectId}
-              currentDialog={currentDialog}
-              openNewTriggerModal={openNewTriggerModal}
-              onCreateDialogComplete={onCreateDialogComplete}
-              onboardingAddCoachMarkRef={onboardingAddCoachMarkRef}
-              showSkillManifestModal={() => setExportSkillModalVisible(true)}
-            />
-          )}
+          <ToolBar
+            toolbarItems={toolbarItems}
+            actions={actions}
+            projectId={projectId}
+            currentDialog={currentDialog}
+            openNewTriggerModal={openNewTriggerModal}
+            onCreateDialogComplete={onCreateDialogComplete}
+            onboardingAddCoachMarkRef={onboardingAddCoachMarkRef}
+            showSkillManifestModal={() => setExportSkillModalVisible(true)}
+          />
           <Conversation css={editorContainer}>
             <div css={editorWrapper}>
               <div css={visualPanel} ref={visualPanelRef} tabIndex={0}>
@@ -468,6 +466,6 @@ function DesignPage(props) {
       </Suspense>
     </React.Fragment>
   );
-}
+};
 
 export default DesignPage;
diff --git a/Composer/packages/client/src/router.tsx b/Composer/packages/client/src/router.tsx
index 0ca7769756..6d186e3859 100644
--- a/Composer/packages/client/src/router.tsx
+++ b/Composer/packages/client/src/router.tsx
@@ -2,9 +2,9 @@
 // Licensed under the MIT License.
 
 /** @jsx jsx */
-import { jsx } from '@emotion/core';
-import React, { Fragment, useContext, useEffect, Suspense } from 'react';
-import { Router, Redirect } from '@reach/router';
+import { jsx, css } from '@emotion/core';
+import React, { useContext, useEffect, Suspense } from 'react';
+import { Router, Redirect, RouteComponentProps } from '@reach/router';
 
 import { resolveToBasePath } from './utils/fileUtil';
 import { About } from './pages/about';
@@ -28,8 +28,6 @@ const Routes = props => {
     <div css={data}>
       <Suspense fallback={<LoadingSpinner />}>
         <Router basepath={BASEPATH} {...props}>
-          {/* <!-- this is out here, instead of inside ProjectRouter, to allow for a specific place in the DOM. --> */}
-          <DialogRouter path="/bot/:projectId/dialogs/:dialogId/*" {...props} />
           <Redirect
             from="/bot/:projectId/language-generation"
             to="/bot/:projectId/language-generation/common"
@@ -44,6 +42,7 @@ const Routes = props => {
           <Redirect from="/" to={resolveToBasePath(BASEPATH, 'home')} noThrow />
           {/* <Redirect from="/bot/:projectId" to="/bot/:projectId/dialogs/Main" noThrow /> */}
           <ProjectRouter path="/bot/:projectId">
+            <DesignPage path="dialogs/:dialogId/*" />
             <SettingPage path="settings/*" />
             <LUPage path="language-understanding/:dialogId/*" />
             <LGPage path="language-generation/:dialogId/*" />
@@ -61,29 +60,30 @@ const Routes = props => {
   );
 };
 
-const DialogRouter = props => {
-  const match = { dialogId: props.dialogId, projectId: props.projectId };
-  const { actions } = useContext(StoreContext);
+const projectStyle = css`
+  height: 100%;
 
-  useEffect(() => {
-    actions.fetchProjectById(props.projectId);
-  }, []);
+  & > div {
+    height: 100%;
+  }
 
-  return (
-    <Fragment>
-      <DesignPage match={match} {...props} />
-    </Fragment>
-  );
-};
+  label: ProjectRouter;
+`;
 
-const ProjectRouter = props => {
-  const { actions } = useContext(StoreContext);
+const ProjectRouter: React.FC<RouteComponentProps<{ projectId: string }>> = props => {
+  const { actions, state } = useContext(StoreContext);
 
   useEffect(() => {
-    actions.fetchProjectById(props.projectId);
-  }, []);
+    if (state.projectId !== props.projectId) {
+      actions.fetchProjectById(props.projectId);
+    }
+  }, [props.projectId, state.projectId]);
+
+  if (props.projectId !== state.projectId) {
+    return <LoadingSpinner />;
+  }
 
-  return <Fragment>{props.children}</Fragment>;
+  return <div css={projectStyle}>{props.children}</div>;
 };
 
 export default Routes;
diff --git a/Composer/packages/client/src/styles.ts b/Composer/packages/client/src/styles.ts
index 8da2658719..ef089f7763 100644
--- a/Composer/packages/client/src/styles.ts
+++ b/Composer/packages/client/src/styles.ts
@@ -53,6 +53,8 @@ export const content = css`
   flex: 1;
   display: flex;
   flex-direction: column;
+
+  label: Content;
 `;
 
 export const showDesign = show => css`
diff --git a/Composer/packages/extensions/adaptive-form/src/components/SchemaField.tsx b/Composer/packages/extensions/adaptive-form/src/components/SchemaField.tsx
index 23de6681cd..7e1487a9e7 100644
--- a/Composer/packages/extensions/adaptive-form/src/components/SchemaField.tsx
+++ b/Composer/packages/extensions/adaptive-form/src/components/SchemaField.tsx
@@ -68,19 +68,20 @@ const SchemaField: React.FC<FieldProps> = props => {
   const deserializedValue = typeof uiOptions?.serializer?.get === 'function' ? uiOptions.serializer.get(value) : value;
 
   const FieldWidget = resolveFieldWidget(schema, uiOptions, pluginConfig);
-  const fieldProps = {
+  const fieldProps: FieldProps = {
     ...rest,
-    name,
-    uiOptions,
+    definitions,
+    description: getUiDescription({ ...props, uiOptions }),
     enumOptions: schema.enum as string[],
+    error: error || undefined,
     label: getUiLabel({ ...props, uiOptions }),
+    name,
+    onChange: handleChange,
     placeholder: getUiPlaceholder({ ...props, uiOptions }),
-    description: getUiDescription({ ...props, uiOptions }),
+    rawErrors: typeof rawErrors?.[name] === 'object' ? rawErrors?.[name] : rawErrors,
     schema,
+    uiOptions,
     value: deserializedValue,
-    error: error || undefined,
-    rawErrors: typeof rawErrors?.[name] === 'object' ? rawErrors?.[name] : rawErrors,
-    onChange: handleChange,
   };
 
   return (
diff --git a/Composer/packages/extensions/adaptive-form/src/components/fields/EditableField.tsx b/Composer/packages/extensions/adaptive-form/src/components/fields/EditableField.tsx
index d759545824..28dc29cdaf 100644
--- a/Composer/packages/extensions/adaptive-form/src/components/fields/EditableField.tsx
+++ b/Composer/packages/extensions/adaptive-form/src/components/fields/EditableField.tsx
@@ -7,7 +7,7 @@ import { NeutralColors } from '@uifabric/fluent-theme';
 import { mergeStyleSets } from '@uifabric/styling';
 import { FieldProps } from '@bfc/extension';
 
-interface EditableFieldProps extends FieldProps {
+interface EditableFieldProps extends Omit<FieldProps, 'definitions'> {
   fontSize?: string;
   styles?: Partial<ITextFieldStyles>;
   transparentBorder?: boolean;
diff --git a/Composer/packages/extensions/adaptive-form/src/utils/resolveBaseSchema.ts b/Composer/packages/extensions/adaptive-form/src/utils/resolveBaseSchema.ts
index 46d3800427..4807047c10 100644
--- a/Composer/packages/extensions/adaptive-form/src/utils/resolveBaseSchema.ts
+++ b/Composer/packages/extensions/adaptive-form/src/utils/resolveBaseSchema.ts
@@ -6,7 +6,8 @@ import { MicrosoftAdaptiveDialog } from '@bfc/shared';
 import { resolveRef } from './resolveRef';
 
 export function resolveBaseSchema(schema: JSONSchema7, data: MicrosoftAdaptiveDialog): JSONSchema7 | undefined {
-  if (schema.definitions && schema.definitions?.[data.$kind] && schema.definitions?.[data.$kind] !== true) {
-    return resolveRef(schema.definitions?.[data.$kind], schema.definitions);
+  const defSchema = schema.definitions?.[data.$kind];
+  if (defSchema && typeof defSchema === 'object') {
+    return resolveRef(defSchema, schema.definitions);
   }
 }
diff --git a/Composer/packages/extensions/adaptive-form/src/utils/resolveRef.ts b/Composer/packages/extensions/adaptive-form/src/utils/resolveRef.ts
index 46c9f13723..ef9b35162e 100644
--- a/Composer/packages/extensions/adaptive-form/src/utils/resolveRef.ts
+++ b/Composer/packages/extensions/adaptive-form/src/utils/resolveRef.ts
@@ -8,7 +8,7 @@ export function resolveRef(
 ): JSONSchema7 {
   if (typeof schema?.$ref === 'string') {
     const defName = schema.$ref.replace('#/definitions/', '');
-    const defSchema = typeof definitions?.[defName] === 'object' ? (definitions?.[defName] as JSONSchema7) : {};
+    const defSchema = typeof definitions[defName] === 'object' ? (definitions[defName] as JSONSchema7) : {};
 
     const resolvedSchema = {
       ...defSchema,
diff --git a/Composer/packages/extensions/extension/src/types/form.ts b/Composer/packages/extensions/extension/src/types/form.ts
index b0ce5a50ed..4374fb8e38 100644
--- a/Composer/packages/extensions/extension/src/types/form.ts
+++ b/Composer/packages/extensions/extension/src/types/form.ts
@@ -34,7 +34,7 @@ export type ChangeHandler<T = any> = (newValue?: T) => void;
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export interface FieldProps<T = any> {
   className?: string;
-  definitions?: { [key: string]: JSONSchema7Definition };
+  definitions: { [key: string]: JSONSchema7Definition } | undefined;
   depth: number;
   description?: string;
   disabled?: boolean;
diff --git a/Composer/packages/ui-plugins/prompts/src/PromptField/index.tsx b/Composer/packages/ui-plugins/prompts/src/PromptField/index.tsx
index 80eeca4fcb..dec06cbe32 100644
--- a/Composer/packages/ui-plugins/prompts/src/PromptField/index.tsx
+++ b/Composer/packages/ui-plugins/prompts/src/PromptField/index.tsx
@@ -72,6 +72,7 @@ const PromptField: React.FC<FieldProps> = props => {
               uiOptions={props.uiOptions.properties?.[f] || {}}
               value={props.value?.[f]}
               onChange={updateField(f)}
+              definitions={props.definitions}
             />
           ))}
         </PivotItem>
diff --git a/Composer/packages/ui-plugins/select-skill-dialog/src/BeginSkillDialogField.tsx b/Composer/packages/ui-plugins/select-skill-dialog/src/BeginSkillDialogField.tsx
index 03c69c993d..4cc3bebfa4 100644
--- a/Composer/packages/ui-plugins/select-skill-dialog/src/BeginSkillDialogField.tsx
+++ b/Composer/packages/ui-plugins/select-skill-dialog/src/BeginSkillDialogField.tsx
@@ -10,7 +10,7 @@ import formatMessage from 'format-message';
 import { SkillEndpointField } from './SkillEndpointField';
 
 export const BeginSkillDialogField: React.FC<FieldProps> = props => {
-  const { depth, id, schema, uiOptions, value, onChange } = props;
+  const { depth, id, schema, uiOptions, value, onChange, definitions } = props;
   const { projectId, skills = [] } = useShellApi();
 
   const manifest = useMemo(() => skills.find(({ manifestUrl }) => manifestUrl === value.id), [skills, value.id]);
@@ -52,6 +52,7 @@ export const BeginSkillDialogField: React.FC<FieldProps> = props => {
         uiOptions={uiOptions.properties?.id || {}}
         value={value?.id}
         onChange={handleIdChange}
+        definitions={definitions}
       />
       <SkillEndpointField
         depth={depth + 1}
@@ -63,6 +64,7 @@ export const BeginSkillDialogField: React.FC<FieldProps> = props => {
         uiOptions={skillEndpointUiSchema}
         value={value?.skillEndpoint}
         onChange={handleEndpointChange}
+        definitions={definitions}
       />
       <Link href={`/bot/${projectId}/skills`} styles={{ root: { fontSize: '12px', padding: '0 16px' } }}>
         {formatMessage('Open Skills page for configuration details')}