fix: LG/LU editing performance optimize (microsoft#3738)

* update luUtil interface * update * use structure luFile do CRUD * refine lgUtile interface * update worker * update lu package * update lg package * update parse line number * log * parse still use worker * lint fix * clean up * Update Composer/packages/client/src/recoilModel/dispatchers/lg.ts Co-authored-by: Andy Brown <[email protected]> * Update Composer/packages/client/src/recoilModel/dispatchers/lg.ts Co-authored-by: Andy Brown <[email protected]> * Update Composer/packages/client/src/recoilModel/dispatchers/lg.ts Co-authored-by: Andy Brown <[email protected]> * Update Composer/packages/client/src/recoilModel/dispatchers/lg.ts Co-authored-by: Andy Brown <[email protected]> * Update Composer/packages/client/src/recoilModel/dispatchers/lg.ts Co-authored-by: Andy Brown <[email protected]> * wrap with formatMessage Co-authored-by: Chris Whitten <[email protected]> Co-authored-by: Andy Brown <[email protected]>
LouisEugeneMSFT · Jul 31, 2020 · ba18903 · ba18903
1 parent 1c49724
commit ba18903
Show file tree

Hide file tree

Showing 25 changed files with 476 additions and 444 deletions.
diff --git a/Composer/packages/client/package.json b/Composer/packages/client/package.json
@@ -32,15 +32,13 @@
     "@bfc/ui-plugin-select-dialog": "*",
     "@bfc/ui-plugin-select-skill-dialog": "*",
     "@emotion/core": "^10.0.27",
-    "@microsoft/bf-lu": "^4.10.0-preview.141651",
     "@reach/router": "^1.2.1",
     "@uifabric/fluent-theme": "^7.1.107",
     "@uifabric/icons": "^7.3.59",
     "@uifabric/react-hooks": "^7.4.12",
     "@uifabric/styling": "^7.13.7",
     "axios": "^0.19.2",
     "babel-plugin-extract-format-message": "^6.2.3",
-    "botbuilder-lg": "4.10.0-preview-147186",
     "format-message": "^6.2.3",
     "immer": "^5.2.0",
     "jwt-decode": "^2.2.0",
@@ -56,8 +54,8 @@
     "react-frame-component": "^4.0.2",
     "react-timeago": "^4.4.0",
     "recoil": "^0.0.10",
-    "webpack-bundle-analyzer": "^3.8.0",
-    "styled-components": "^4.1.3"
+    "styled-components": "^4.1.3",
+    "webpack-bundle-analyzer": "^3.8.0"
   },
   "browserslist": [
     ">0.2%",
@@ -66,7 +64,6 @@
     "not op_mini all"
   ],
   "devDependencies": {
-    "@types/recoil": "^0.0.1",
     "@babel/cli": "7.2.3",
     "@babel/core": "7.3.4",
     "@babel/runtime": "7.3.4",
@@ -78,6 +75,7 @@
     "@types/reach__router": "^1.2.4",
     "@types/react": "16.9.23",
     "@types/react-dom": "16.9.5",
+    "@types/recoil": "^0.0.1",
     "@types/webpack-env": "^1.15.2",
     "babel-core": "7.0.0-bridge.0",
     "babel-eslint": "10.1.0",

diff --git a/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx b/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx
@@ -14,7 +14,7 @@ import { Dropdown } from 'office-ui-fabric-react/lib/Dropdown';
 import { TextField } from 'office-ui-fabric-react/lib/TextField';
 import { luIndexer, combineMessage } from '@bfc/indexers';
 import { PlaceHolderSectionName } from '@bfc/indexers/lib/utils/luUtil';
-import { DialogInfo, SDKKinds } from '@bfc/shared';
+import { DialogInfo, SDKKinds, LuIntentSection } from '@bfc/shared';
 import { LuEditor, inlineModePlaceholder } from '@bfc/code-editor';
 import { IComboBoxOption } from 'office-ui-fabric-react/lib/ComboBox';
 import { useRecoilValue } from 'recoil';
@@ -34,14 +34,7 @@ import {
   getActivityTypes,
   regexRecognizerKey,
 } from '../../utils/dialogUtil';
-import { addIntent } from '../../utils/luUtil';
-import {
-  dialogsState,
-  luFilesState,
-  localeState,
-  projectIdState,
-  schemasState,
-} from '../../recoilModel/atoms/botState';
+import { dialogsState, projectIdState, schemasState } from '../../recoilModel/atoms/botState';
 import { userSettingsState } from '../../recoilModel';
 import { nameRegex } from '../../constants';
 
@@ -206,18 +199,17 @@ interface TriggerCreationModalProps {
   dialogId: string;
   isOpen: boolean;
   onDismiss: () => void;
-  onSubmit: (dialog: DialogInfo, luFilePayload?: LuFilePayload) => void;
+  onSubmit: (dialog: DialogInfo, intent?: LuIntentSection) => void;
 }
 
 export const TriggerCreationModal: React.FC<TriggerCreationModalProps> = (props) => {
   const { isOpen, onDismiss, onSubmit, dialogId } = props;
   const dialogs = useRecoilValue(dialogsState);
-  const luFiles = useRecoilValue(luFilesState);
-  const locale = useRecoilValue(localeState);
+
   const projectId = useRecoilValue(projectIdState);
   const schemas = useRecoilValue(schemasState);
   const userSettings = useRecoilValue(userSettingsState);
-  const luFile = luFiles.find(({ id }) => id === `${dialogId}.${locale}`);
+
   const dialogFile = dialogs.find((dialog) => dialog.id === dialogId);
   const isRegEx = (dialogFile?.content?.recognizer?.$kind ?? '') === regexRecognizerKey;
   const regexIntents = dialogFile?.content?.recognizer?.intents ?? [];
@@ -268,16 +260,10 @@ export const TriggerCreationModal: React.FC<TriggerCreationModalProps> = (props)
       });
       return;
     }
-    const content = luFile?.content ?? '';
-    const luFileId = luFile?.id || `${dialogId}.${locale}`;
     const newDialog = generateNewDialog(dialogs, dialogId, formData, schemas.sdk?.content);
     if (formData.$kind === intentTypeKey && !isRegEx) {
-      const newContent = addIntent(content, { Name: formData.intent, Body: formData.triggerPhrases });
-      const updateLuFile = {
-        id: luFileId,
-        content: newContent,
-      };
-      onSubmit(newDialog, updateLuFile);
+      const newIntent = { Name: formData.intent, Body: formData.triggerPhrases };
+      onSubmit(newDialog, newIntent);
     } else {
       onSubmit(newDialog);
     }

diff --git a/Composer/packages/client/src/pages/design/DesignPage.tsx b/Composer/packages/client/src/pages/design/DesignPage.tsx
@@ -8,7 +8,7 @@ import { Breadcrumb, IBreadcrumbItem } from 'office-ui-fabric-react/lib/Breadcru
 import formatMessage from 'format-message';
 import { globalHistory, RouteComponentProps } from '@reach/router';
 import get from 'lodash/get';
-import { DialogFactory, SDKKinds, DialogInfo, PromptTab } from '@bfc/shared';
+import { DialogFactory, SDKKinds, DialogInfo, PromptTab, LuIntentSection } from '@bfc/shared';
 import { ActionButton } from 'office-ui-fabric-react/lib/Button';
 import { JsonEditor } from '@bfc/code-editor';
 import { useTriggerApi } from '@bfc/extension';
@@ -18,7 +18,6 @@ import { LoadingSpinner } from '../../components/LoadingSpinner';
 import { TestController } from '../../components/TestController/TestController';
 import { DialogDeleting } from '../../constants';
 import { createSelectedPath, deleteTrigger, getbreadcrumbLabel } from '../../utils/dialogUtil';
-import { LuFilePayload } from '../../components/ProjectTree/TriggerCreationModal';
 import { Conversation } from '../../components/Conversation';
 import { dialogStyle } from '../../components/Modal/dialogStyle';
 import { OpenConfirmModal } from '../../components/Modal/ConfirmDialog';
@@ -43,6 +42,8 @@ import {
   skillsState,
   actionsSeedState,
   userSettingsState,
+  luFilesState,
+  localeState,
 } from '../../recoilModel';
 
 import { VisualEditorAPI } from './FrameAPI';
@@ -112,6 +113,9 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
   const skills = useRecoilValue(skillsState);
   const actionsSeed = useRecoilValue(actionsSeedState);
   const userSettings = useRecoilValue(userSettingsState);
+  const luFiles = useRecoilValue(luFilesState);
+  const locale = useRecoilValue(localeState);
+
   const {
     removeDialog,
     updateDialog,
@@ -124,13 +128,14 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
     selectTo,
     selectAndFocus,
     addSkillDialogCancel,
-    updateLuFile,
+    createLuIntent,
     updateSkill,
     exportToZip,
     onboardingAddCoachMarkRef,
   } = useRecoilValue(dispatcherState);
 
   const { location, dialogId } = props;
+  const luFile = luFiles.find(({ id }) => id === `${dialogId}.${locale}`);
   const params = new URLSearchParams(location?.search);
   const selected = params.get('selected') || '';
   const [triggerModalVisible, setTriggerModalVisibility] = useState(false);
@@ -190,19 +195,14 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
     setTriggerModalVisibility(true);
   };
 
-  const onTriggerCreationSubmit = (dialog: DialogInfo, luFile?: LuFilePayload) => {
+  const onTriggerCreationSubmit = (dialog: DialogInfo, intent?: LuIntentSection) => {
     const dialogPayload = {
       id: dialog.id,
       projectId,
       content: dialog.content,
     };
-    if (luFile) {
-      const luFilePayload = {
-        id: luFile.id,
-        content: luFile.content,
-        projectId,
-      };
-      updateLuFile(luFilePayload);
+    if (luFile && intent) {
+      createLuIntent({ id: luFile.id, intent });
     }
 
     const index = get(dialog, 'content.triggers', []).length - 1;

diff --git a/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx b/Composer/packages/client/src/recoilModel/dispatchers/__tests__/lu.test.tsx
@@ -5,6 +5,7 @@ import { useRecoilState } from 'recoil';
 import { LuIntentSection, LuFile } from '@bfc/shared';
 import { useRecoilValue } from 'recoil';
 import { act } from '@bfc/test-utils/lib/hooks';
+import { luUtil } from '@bfc/indexers';
 
 import { renderRecoilHook } from '../../../../__tests__/testUtils';
 import { luFilesState } from '../../atoms';
@@ -22,12 +23,13 @@ jest.mock('../../parsers/luWorker', () => {
     removeIntents: require('@bfc/indexers/lib/utils/luUtil').removeIntents,
   };
 });
-const luFiles = [
-  {
-    id: 'common.en-us',
-    content: `\r\n# Hello\r\n-hi`,
-  },
-] as LuFile[];
+
+const file1 = {
+  id: 'common.en-us',
+  content: `\r\n# Hello\r\n-hi`,
+};
+
+const luFiles = [luUtil.parse(file1.id, file1.content)] as LuFile[];
 
 const getLuIntent = (Name, Body): LuIntentSection =>
   ({
@@ -96,7 +98,7 @@ describe('Lu dispatcher', () => {
     expect(renderedComponent.current.luFiles[0].content).toMatch(/-IntentValue/);
   });
 
-  it('should remove a lg template', async () => {
+  it('should remove a lu intent', async () => {
     await act(async () => {
       await dispatcher.removeLuIntent({
         id: luFiles[0].id,

diff --git a/Composer/packages/client/src/recoilModel/dispatchers/lg.ts b/Composer/packages/client/src/recoilModel/dispatchers/lg.ts
@@ -4,6 +4,7 @@
 import { LgTemplate, LgFile, importResolverGenerator } from '@bfc/shared';
 import { useRecoilCallback, CallbackInterface } from 'recoil';
 import differenceBy from 'lodash/differenceBy';
+import formatMessage from 'format-message';
 
 import { getBaseName, getExtension } from '../../utils/fileUtil';
 
@@ -20,13 +21,13 @@ const initialBody = '- ';
 
 export const updateLgFileState = async (
   callbackHelpers: CallbackInterface,
-  { id, content }: { id: string; content: string }
+  { id, content, updatedFile }: { id: string; content: string; updatedFile?: LgFile }
 ) => {
   const { set, snapshot } = callbackHelpers;
   const lgFiles = await snapshot.getPromise(lgFilesState);
   const dialogId = getBaseName(id);
   const locale = getExtension(id);
-  const updatedLgFile = (await LgWorker.parse(id, content, lgFiles)) as LgFile;
+  const updatedLgFile = updatedFile || ((await LgWorker.parse(id, content, lgFiles)) as LgFile);
   const originLgFile = lgFiles.find((file) => id === file.id);
   const sameIdOtherLocaleFiles = lgFiles.filter((file) => {
     const fileDialogId = getBaseName(file.id);
@@ -35,7 +36,7 @@ export const updateLgFileState = async (
   });
 
   if (!originLgFile) {
-    throw new Error('origin lg file not found in store');
+    throw new Error(formatMessage('origin lg file not found in store'));
   }
 
   const changes: LgFile[] = [updatedLgFile];
@@ -58,10 +59,9 @@ export const updateLgFileState = async (
   if (onlyAdds || onlyDeletes) {
     for (const file of sameIdOtherLocaleFiles) {
       const lgImportResolver = importResolverGenerator(lgFiles, '.lg', getExtension(file.id));
-      let newLgFile = lgUtil.addTemplates(file.id, file.content, addedTemplates, lgImportResolver);
+      let newLgFile = lgUtil.addTemplates(file, addedTemplates, lgImportResolver);
       newLgFile = lgUtil.removeTemplates(
-        file.id,
-        newLgFile.content,
+        newLgFile,
         deletedTemplates.map(({ name }) => name),
         lgImportResolver
       );
@@ -92,7 +92,7 @@ export const createLgFileState = async (
   const { languages } = await snapshot.getPromise(settingsState);
   const createdLgId = `${id}.${locale}`;
   if (lgFiles.find((lg) => lg.id === createdLgId)) {
-    throw new Error('lg file already exist');
+    throw new Error(formatMessage('lg file already exist'));
   }
   // slot with common.lg import
   let lgInitialContent = '';
@@ -101,7 +101,7 @@ export const createLgFileState = async (
     lgInitialContent = `[import](common.lg)`;
   }
   content = [lgInitialContent, content].join('\n');
-  const createdLgFile = (await LgWorker.parse(createdLgId, content, lgFiles)) as LgFile;
+  const createdLgFile = lgUtil.parse(createdLgId, content, lgFiles);
   const changes: LgFile[] = [];
 
   // copy to other locales
@@ -155,39 +155,53 @@ export const lgDispatcher = () => {
     }) => {
       const { snapshot } = callbackHelpers;
       const lgFiles = await snapshot.getPromise(lgFilesState);
-      let content = lgFiles.find((file) => file.id === id)?.content ?? '';
-      content = lgUtil.updateTemplate(id, content, templateName, template, lgFileResolver(lgFiles)).content;
-      await updateLgFileState(callbackHelpers, { id, content });
+      const lgFile = lgFiles.find((file) => file.id === id);
+      if (!lgFile) {
+        throw new Error(formatMessage('lg file {id} does not exist.', { id }));
+      }
+      const updatedFile = lgUtil.updateTemplate(lgFile, templateName, template, lgFileResolver(lgFiles));
+      await updateLgFileState(callbackHelpers, { id, content: updatedFile.content, updatedFile });
     }
   );
 
   const createLgTemplate = useRecoilCallback(
     (callbackHelpers: CallbackInterface) => async ({ id, template }: { id: string; template: LgTemplate }) => {
       const { snapshot } = callbackHelpers;
       const lgFiles = await snapshot.getPromise(lgFilesState);
-      let content = lgFiles.find((file) => file.id === id)?.content ?? '';
-      content = lgUtil.addTemplate(id, content, template, lgFileResolver(lgFiles)).content;
-      await updateLgFileState(callbackHelpers, { id, content });
+      const lgFile = lgFiles.find((file) => file.id === id);
+      if (!lgFile) {
+        throw new Error(formatMessage('lg file {id} does not exist.', { id }));
+      }
+
+      const updatedFile = lgUtil.addTemplate(lgFile, template, lgFileResolver(lgFiles));
+      await updateLgFileState(callbackHelpers, { id, updatedFile, content: updatedFile.content });
     }
   );
 
   const removeLgTemplate = useRecoilCallback(
     (callbackHelpers: CallbackInterface) => async ({ id, templateName }: { id: string; templateName: string }) => {
       const { snapshot } = callbackHelpers;
       const lgFiles = await snapshot.getPromise(lgFilesState);
-      let content = lgFiles.find((file) => file.id === id)?.content ?? '';
-      content = lgUtil.removeTemplate(id, content, templateName, lgFileResolver(lgFiles)).content;
-      await updateLgFileState(callbackHelpers, { id, content });
+      const lgFile = lgFiles.find((file) => file.id === id);
+      if (!lgFile) {
+        throw new Error(formatMessage('lg file {id} does not exist.', { id }));
+      }
+
+      const updatedFile = lgUtil.removeTemplate(lgFile, templateName, lgFileResolver(lgFiles));
+      await updateLgFileState(callbackHelpers, { id, updatedFile, content: updatedFile.content });
     }
   );
 
   const removeLgTemplates = useRecoilCallback(
     (callbackHelpers: CallbackInterface) => async ({ id, templateNames }: { id: string; templateNames: string[] }) => {
       const { snapshot } = callbackHelpers;
       const lgFiles = await snapshot.getPromise(lgFilesState);
-      let content = lgFiles.find((file) => file.id === id)?.content ?? '';
-      content = lgUtil.removeTemplates(id, content, templateNames, lgFileResolver(lgFiles)).content;
-      await updateLgFileState(callbackHelpers, { id, content });
+      const lgFile = lgFiles.find((file) => file.id === id);
+      if (!lgFile) {
+        throw new Error(formatMessage('lg file {id} does not exist.', { id }));
+      }
+      const updatedFile = lgUtil.removeTemplates(lgFile, templateNames, lgFileResolver(lgFiles));
+      await updateLgFileState(callbackHelpers, { id, updatedFile, content: updatedFile.content });
     }
   );
 
@@ -203,9 +217,12 @@ export const lgDispatcher = () => {
     }) => {
       const { snapshot } = callbackHelpers;
       const lgFiles = await snapshot.getPromise(lgFilesState);
-      let content = lgFiles.find((file) => file.id === id)?.content ?? '';
-      content = lgUtil.copyTemplate(id, content, fromTemplateName, toTemplateName, lgFileResolver(lgFiles)).content;
-      await updateLgFileState(callbackHelpers, { id, content });
+      const lgFile = lgFiles.find((file) => file.id === id);
+      if (!lgFile) {
+        throw new Error(formatMessage('lg file {id} does not exist.', { id }));
+      }
+      const updatedFile = lgUtil.copyTemplate(lgFile, fromTemplateName, toTemplateName, lgFileResolver(lgFiles));
+      await updateLgFileState(callbackHelpers, { id, updatedFile, content: updatedFile.content });
     }
   );