From fd6b3c883809ee3bd49e02b902e80b5cad67923b Mon Sep 17 00:00:00 2001 From: leileizhang Date: Thu, 18 Jun 2020 03:41:48 +0800 Subject: [PATCH] fix: improve the user experience about lg editor (#3337) * move lg api into worker * output worker files with readable names * update the version of botbuild-lg * remove promise in parse event * refacotr lg worker to catch all errors * add worker for lg lsp * add a null check when rejecting * dissallow node integration in worker threads * remove request_light * update worker file naming * fix unit test * set asar = false * add force exit for jest * move node_module to unpack folder * Swapped out server side worker with child process. * fix unit tests in lg lsp * clean the code * only support test env in lg lsp * fix some comments * update the dev dependency Co-authored-by: Andy Brown Co-authored-by: Chris Whitten Co-authored-by: Tony Anziano --- Composer/package.json | 2 +- .../client/__tests__/shell/lgApi.test.tsx | 2 +- .../packages/client/config/webpack.config.js | 5 +- Composer/packages/client/package.json | 3 +- .../pages/language-generation/code-editor.tsx | 1 - Composer/packages/client/src/shell/lgApi.ts | 11 +- .../packages/client/src/store/action/lg.ts | 11 +- .../client/src/store/parsers/baseWorker.ts | 6 +- .../client/src/store/parsers/indexer.ts | 6 +- .../client/src/store/parsers/lgWorker.ts | 40 ++++++- .../client/src/store/parsers/luWorker.ts | 18 +-- .../client/src/store/parsers/types.ts | 45 +++++++- .../store/parsers/workers/lgParser.worker.ts | 99 ++++++++++++++-- .../store/parsers/workers/luParser.worker.ts | 6 +- .../packages/client/src/utils/dialogUtil.ts | 11 -- .../electron-server/src/electronWindow.ts | 1 + Composer/packages/lib/indexers/package.json | 4 +- Composer/packages/server/src/server.ts | 4 +- .../packages/server/src/services/project.ts | 7 +- .../__tests__/LGServer.test.ts | 8 ++ .../__tests__/helpers/server.ts | 14 +-- .../language-generation/demo/src/server.ts | 4 +- .../language-generation/package.json | 5 +- .../language-generation/src/LGServer.ts | 108 +++++++----------- .../language-generation/src/lgParser.ts | 57 +++++++++ .../language-generation/src/lgWorker.ts | 37 ++++++ .../language-generation/src/utils.ts | 2 +- .../packages/ui-plugins/lg/src/LgField.tsx | 2 +- Composer/yarn.lock | 31 ++--- 29 files changed, 368 insertions(+), 182 deletions(-) create mode 100644 Composer/packages/tools/language-servers/language-generation/src/lgParser.ts create mode 100644 Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts diff --git a/Composer/package.json b/Composer/package.json index 25e5749784..62cb05b54a 100644 --- a/Composer/package.json +++ b/Composer/package.json @@ -51,7 +51,7 @@ "runtime": "cd ../runtime/dotnet/azurewebapp && dotnet build && dotnet run", "test": "yarn typecheck && jest", "test:watch": "yarn typecheck && jest --watch", - "test:coverage": "yarn test --coverage --no-cache --reporters=default", + "test:coverage": "yarn test --coverage --no-cache --forceExit --reporters=default", "test:integration": "cypress run --browser edge", "test:integration:start-server": "node scripts/e2e.js", "test:integration:open": "cypress open", diff --git a/Composer/packages/client/__tests__/shell/lgApi.test.tsx b/Composer/packages/client/__tests__/shell/lgApi.test.tsx index e277a84d0c..67b733bf71 100644 --- a/Composer/packages/client/__tests__/shell/lgApi.test.tsx +++ b/Composer/packages/client/__tests__/shell/lgApi.test.tsx @@ -60,7 +60,7 @@ describe('use lgApi hooks', () => { const { result } = renderHook(() => useLgApi()); result.current.updateLgTemplate('test.en-us', 'bar', 'update'); - + result.current.updateLgTemplate.flush(); expect(actions.updateLgTemplate).toBeCalledTimes(1); const arg = { file: { diff --git a/Composer/packages/client/config/webpack.config.js b/Composer/packages/client/config/webpack.config.js index 585d0b57ac..cbf2051ab8 100644 --- a/Composer/packages/client/config/webpack.config.js +++ b/Composer/packages/client/config/webpack.config.js @@ -274,7 +274,10 @@ module.exports = function (webpackEnv) { }, { test: /\.worker\.ts$/, - use: { loader: 'worker-loader' }, + use: { + loader: 'worker-loader', + options: { name: isEnvProduction ? '[hash].worker.js' : '[name].js' }, + }, }, { // "oneOf" will traverse all following loaders until one will diff --git a/Composer/packages/client/package.json b/Composer/packages/client/package.json index 1e00dc8de4..58ecdd7350 100644 --- a/Composer/packages/client/package.json +++ b/Composer/packages/client/package.json @@ -37,9 +37,8 @@ "@uifabric/icons": "^7.3.4", "@uifabric/react-hooks": "^7.3.4", "@uifabric/styling": "^7.10.4", - "adaptive-expressions": "^4.10.0-preview-133287", "axios": "^0.19.2", - "botbuilder-lg": "^4.10.0-preview-133287", + "botbuilder-lg": "4.10.0-preview-135858", "format-message": "^6.2.3", "immer": "^5.2.0", "jwt-decode": "^2.2.0", diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index b2bf6d86ee..6ec8b5f3b7 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -105,7 +105,6 @@ const CodeEditor: React.FC = (props) => { const _onChange = useCallback( (value) => { - setContent(value); if (!file) return; if (inlineMode) { if (!template) return; diff --git a/Composer/packages/client/src/shell/lgApi.ts b/Composer/packages/client/src/shell/lgApi.ts index dbe9b16665..d71fed6668 100644 --- a/Composer/packages/client/src/shell/lgApi.ts +++ b/Composer/packages/client/src/shell/lgApi.ts @@ -3,14 +3,11 @@ import { useEffect, useState } from 'react'; import { LgFile } from '@bfc/shared'; -import throttle from 'lodash/throttle'; +import debounce from 'lodash/debounce'; -import * as lgUtil from '../utils/lgUtil'; import { State, BoundActionHandlers } from '../store/types'; import { useStoreContext } from '../hooks/useStoreContext'; -const createThrottledFunc = (fn) => throttle(fn, 1000, { leading: true, trailing: true }); - function createLgApi(state: State, actions: BoundActionHandlers, lgFileResolver: (id: string) => LgFile | undefined) { const getLgTemplates = (id) => { if (id === undefined) throw new Error('must have a file id'); @@ -28,9 +25,7 @@ function createLgApi(state: State, actions: BoundActionHandlers, lgFileResolver: const projectId = state.projectId; - lgUtil.checkSingleLgTemplate(template); - - await actions.updateLgTemplate({ + return actions.updateLgTemplate({ file, projectId, templateName, @@ -82,7 +77,7 @@ function createLgApi(state: State, actions: BoundActionHandlers, lgFileResolver: return { addLgTemplate: updateLgTemplate, getLgTemplates, - updateLgTemplate: createThrottledFunc(updateLgTemplate), + updateLgTemplate: debounce(updateLgTemplate, 250), removeLgTemplate, removeLgTemplates, copyLgTemplate, diff --git a/Composer/packages/client/src/store/action/lg.ts b/Composer/packages/client/src/store/action/lg.ts index ef8f4ce732..82427344a9 100644 --- a/Composer/packages/client/src/store/action/lg.ts +++ b/Composer/packages/client/src/store/action/lg.ts @@ -4,7 +4,6 @@ import clonedeep from 'lodash/cloneDeep'; import { LgFile } from '@bfc/shared'; import { ActionTypes } from '../../constants'; -import * as lgUtil from '../../utils/lgUtil'; import { undoable } from '../middlewares/undo'; import { ActionCreator, State, Store } from '../types'; import LgWorker from '../parsers/lgWorker'; @@ -48,26 +47,26 @@ export const undoableUpdateLgFile = undoable( ); export const updateLgTemplate: ActionCreator = async (store, { file, projectId, templateName, template }) => { - const newContent = lgUtil.updateTemplate(file.content, templateName, template); + const newContent = await LgWorker.updateTemplate(file.content, templateName, template); return await undoableUpdateLgFile(store, { id: file.id, projectId, content: newContent }); }; export const createLgTemplate: ActionCreator = async (store, { file, projectId, template }) => { - const newContent = lgUtil.addTemplate(file.content, template); + const newContent = await LgWorker.addTemplate(file.content, template); return await undoableUpdateLgFile(store, { id: file.id, projectId, content: newContent }); }; export const removeLgTemplate: ActionCreator = async (store, { file, projectId, templateName }) => { - const newContent = lgUtil.removeTemplate(file.content, templateName); + const newContent = await LgWorker.removeTemplate(file.content, templateName); return await undoableUpdateLgFile(store, { id: file.id, projectId, content: newContent }); }; export const removeLgTemplates: ActionCreator = async (store, { file, projectId, templateNames }) => { - const newContent = lgUtil.removeTemplates(file.content, templateNames); + const newContent = await LgWorker.removeAllTemplates(file.content, templateNames); return await undoableUpdateLgFile(store, { id: file.id, projectId, content: newContent }); }; export const copyLgTemplate: ActionCreator = async (store, { file, fromTemplateName, toTemplateName, projectId }) => { - const newContent = lgUtil.copyTemplate(file.content, fromTemplateName, toTemplateName); + const newContent = await LgWorker.copyTemplate(file.content, fromTemplateName, toTemplateName); return await undoableUpdateLgFile(store, { id: file.id, content: newContent, projectId }); }; diff --git a/Composer/packages/client/src/store/parsers/baseWorker.ts b/Composer/packages/client/src/store/parsers/baseWorker.ts index 38d7091abd..616c9d41bf 100644 --- a/Composer/packages/client/src/store/parsers/baseWorker.ts +++ b/Composer/packages/client/src/store/parsers/baseWorker.ts @@ -12,7 +12,7 @@ interface WorkerMsg { } // Wrapper class -export class BaseWorker { +export class BaseWorker { private worker: Worker; private resolves = {}; private rejects = {}; @@ -22,9 +22,9 @@ export class BaseWorker { this.worker.onmessage = this.handleMsg.bind(this); } - public sendMsg(payload: Payload) { + public sendMsg(type: ActionType, payload: Payload) { const msgId = uniqueId(); - const msg = { id: msgId, payload }; + const msg = { id: msgId, type, payload }; return new Promise((resolve, reject) => { // save callbacks for later this.resolves[msgId] = resolve; diff --git a/Composer/packages/client/src/store/parsers/indexer.ts b/Composer/packages/client/src/store/parsers/indexer.ts index 565083d44b..7a5a817b7f 100644 --- a/Composer/packages/client/src/store/parsers/indexer.ts +++ b/Composer/packages/client/src/store/parsers/indexer.ts @@ -4,12 +4,12 @@ import { FileInfo } from '@bfc/shared'; import Worker from './workers/indexer.worker.ts'; import { BaseWorker } from './baseWorker'; -import { IndexPayload } from './types'; +import { IndexPayload, IndexerActionType } from './types'; // Wrapper class -class Indexer extends BaseWorker { +class Indexer extends BaseWorker { index(files: FileInfo, botName: string, schemas: any, locale: string) { - return this.sendMsg({ files, botName, schemas, locale }); + return this.sendMsg(IndexerActionType.Index, { files, botName, schemas, locale }); } } diff --git a/Composer/packages/client/src/store/parsers/lgWorker.ts b/Composer/packages/client/src/store/parsers/lgWorker.ts index 7cb8a7462f..6a79c9a0bf 100644 --- a/Composer/packages/client/src/store/parsers/lgWorker.ts +++ b/Composer/packages/client/src/store/parsers/lgWorker.ts @@ -1,15 +1,47 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { LgFile } from '@bfc/shared'; +import { LgFile, LgTemplate } from '@bfc/shared'; import Worker from './workers/lgParser.worker.ts'; import { BaseWorker } from './baseWorker'; -import { LgPayload } from './types'; +import { + LgActionType, + LgParsePayload, + LgUpdateTemplatePayload, + LgCreateTemplatePayload, + LgRemoveTemplatePayload, + LgRemoveAllTemplatesPayload, + LgCopyTemplatePayload, +} from './types'; // Wrapper class -class LgWorker extends BaseWorker { +class LgWorker extends BaseWorker { parse(targetId: string, content: string, lgFiles: LgFile[]) { - return this.sendMsg({ targetId, content, lgFiles: lgFiles }); + return this.sendMsg(LgActionType.Parse, { targetId, content, lgFiles: lgFiles }); + } + + addTemplate(content: string, template: LgTemplate) { + return this.sendMsg(LgActionType.AddTemplate, { content, template }); + } + + updateTemplate(content: string, templateName: string, template: LgTemplate) { + return this.sendMsg(LgActionType.UpdateTemplate, { content, templateName, template }); + } + + removeTemplate(content: string, templateName: string) { + return this.sendMsg(LgActionType.RemoveTemplate, { content, templateName }); + } + + removeAllTemplates(content: string, templateNames: string[]) { + return this.sendMsg(LgActionType.RemoveAllTemplates, { content, templateNames }); + } + + copyTemplate(content: string, fromTemplateName: string, toTemplateName: string) { + return this.sendMsg(LgActionType.CopyTemplate, { + content, + fromTemplateName, + toTemplateName, + }); } } diff --git a/Composer/packages/client/src/store/parsers/luWorker.ts b/Composer/packages/client/src/store/parsers/luWorker.ts index b2a0f8f85e..58573c6bb1 100644 --- a/Composer/packages/client/src/store/parsers/luWorker.ts +++ b/Composer/packages/client/src/store/parsers/luWorker.ts @@ -7,25 +7,25 @@ import { BaseWorker } from './baseWorker'; import { LuPayload, LuActionType } from './types'; // Wrapper class -class LuWorker extends BaseWorker { +class LuWorker extends BaseWorker { parse(id: string, content: string) { - const payload = { type: LuActionType.Parse, id, content }; - return this.sendMsg(payload); + const payload = { id, content }; + return this.sendMsg(LuActionType.Parse, payload); } addIntent(content: string, intent: LuIntentSection) { - const payload = { type: LuActionType.AddIntent, content, intent }; - return this.sendMsg(payload); + const payload = { content, intent }; + return this.sendMsg(LuActionType.AddIntent, payload); } updateIntent(content: string, intentName: string, intent?: LuIntentSection) { - const payload = { type: LuActionType.UpdateIntent, content, intentName, intent }; - return this.sendMsg(payload); + const payload = { content, intentName, intent }; + return this.sendMsg(LuActionType.UpdateIntent, payload); } removeIntent(content: string, intentName: string) { - const payload = { type: LuActionType.RemoveIntent, content, intentName }; - return this.sendMsg(payload); + const payload = { content, intentName }; + return this.sendMsg(LuActionType.RemoveIntent, payload); } } diff --git a/Composer/packages/client/src/store/parsers/types.ts b/Composer/packages/client/src/store/parsers/types.ts index 63a64bbe83..569b643a40 100644 --- a/Composer/packages/client/src/store/parsers/types.ts +++ b/Composer/packages/client/src/store/parsers/types.ts @@ -1,21 +1,47 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { LuIntentSection, LgFile, FileInfo } from '@bfc/shared'; +import { LuIntentSection, LgFile, FileInfo, LgTemplate } from '@bfc/shared'; export type LuPayload = { - type: LuActionType; content: string; id?: string; intentName?: string; intent?: LuIntentSection; }; -export type LgPayload = { +export type LgParsePayload = { targetId: string; content: string; lgFiles: LgFile[]; }; +export interface LgCreateTemplatePayload { + content: string; + template: LgTemplate; +} + +export interface LgUpdateTemplatePayload { + content: string; + templateName: string; + template: LgTemplate; +} + +export interface LgRemoveTemplatePayload { + content: string; + templateName: string; +} + +export interface LgRemoveAllTemplatesPayload { + content: string; + templateNames: string[]; +} + +export interface LgCopyTemplatePayload { + content: string; + fromTemplateName: string; + toTemplateName: string; +} + export type IndexPayload = { files: FileInfo; botName: string; @@ -29,3 +55,16 @@ export enum LuActionType { UpdateIntent = 'update-intent', RemoveIntent = 'remove-intent', } + +export enum LgActionType { + Parse = 'parse', + AddTemplate = 'add-template', + UpdateTemplate = 'update-template', + RemoveTemplate = 'remove-template', + RemoveAllTemplates = 'remove-all-templates', + CopyTemplate = 'copy-template', +} + +export enum IndexerActionType { + Index = 'index', +} diff --git a/Composer/packages/client/src/store/parsers/workers/lgParser.worker.ts b/Composer/packages/client/src/store/parsers/workers/lgParser.worker.ts index 827a30fd0c..87b8f13920 100644 --- a/Composer/packages/client/src/store/parsers/workers/lgParser.worker.ts +++ b/Composer/packages/client/src/store/parsers/workers/lgParser.worker.ts @@ -3,18 +3,103 @@ import { importResolverGenerator } from '@bfc/shared'; import { lgIndexer } from '@bfc/indexers'; +import * as lgUtil from '../../../utils/lgUtil'; +import { + LgActionType, + LgParsePayload, + LgUpdateTemplatePayload, + LgCreateTemplatePayload, + LgRemoveTemplatePayload, + LgRemoveAllTemplatesPayload, + LgCopyTemplatePayload, +} from '../types'; + const ctx: Worker = self as any; -ctx.onmessage = function (msg) { - const { id, payload } = msg.data; - const { targetId, content, lgFiles } = payload; - const { parse } = lgIndexer; +interface ParseMessage { + id: string; + type: LgActionType.Parse; + payload: LgParsePayload; +} + +interface AddMessage { + id: string; + type: LgActionType.AddTemplate; + payload: LgCreateTemplatePayload; +} + +interface UpdateMessage { + id: string; + type: LgActionType.UpdateTemplate; + payload: LgUpdateTemplatePayload; +} + +interface RemoveMessage { + id: string; + type: LgActionType.RemoveTemplate; + payload: LgRemoveTemplatePayload; +} + +interface RemoveAllMessage { + id: string; + type: LgActionType.RemoveAllTemplates; + payload: LgRemoveAllTemplatesPayload; +} + +interface CopyMessage { + id: string; + type: LgActionType.CopyTemplate; + payload: LgCopyTemplatePayload; +} + +type LgMessageEvent = ParseMessage | AddMessage | UpdateMessage | RemoveMessage | RemoveAllMessage | CopyMessage; + +ctx.onmessage = function (event) { + const msg = event.data as LgMessageEvent; + const id = msg.id; + + let payload: any = null; try { - const lgImportresolver = importResolverGenerator(lgFiles, '.lg'); + switch (msg.type) { + case LgActionType.Parse: { + const { targetId, content, lgFiles } = msg.payload; + const { parse } = lgIndexer; + + const lgImportResolver = importResolverGenerator(lgFiles, '.lg'); - const { templates, diagnostics } = parse(content, targetId, lgImportresolver); + const { templates, diagnostics } = parse(content, targetId, lgImportResolver); + payload = { id: targetId, content, templates, diagnostics }; + break; + } + case LgActionType.AddTemplate: { + const { content, template } = msg.payload; + payload = lgUtil.addTemplate(content, template); + break; + } + case LgActionType.UpdateTemplate: { + const { content, templateName, template } = msg.payload; + lgUtil.checkSingleLgTemplate(template); + payload = lgUtil.updateTemplate(content, templateName, template); + break; + } + case LgActionType.RemoveTemplate: { + const { content, templateName } = msg.payload; + payload = lgUtil.removeTemplate(content, templateName); + break; + } + case LgActionType.RemoveAllTemplates: { + const { content, templateNames } = msg.payload; + payload = lgUtil.removeTemplates(content, templateNames); + break; + } + case LgActionType.CopyTemplate: { + const { content, toTemplateName, fromTemplateName } = msg.payload; + payload = lgUtil.copyTemplate(content, fromTemplateName, toTemplateName); + break; + } + } - ctx.postMessage({ id, payload: { id: targetId, content, templates, diagnostics } }); + ctx.postMessage({ id, payload }); } catch (error) { ctx.postMessage({ id, error }); } diff --git a/Composer/packages/client/src/store/parsers/workers/luParser.worker.ts b/Composer/packages/client/src/store/parsers/workers/luParser.worker.ts index b46be939c6..0256c070e8 100644 --- a/Composer/packages/client/src/store/parsers/workers/luParser.worker.ts +++ b/Composer/packages/client/src/store/parsers/workers/luParser.worker.ts @@ -3,7 +3,7 @@ import { luIndexer } from '@bfc/indexers'; import * as luUtil from '@bfc/indexers/lib/utils/luUtil'; -import { LuActionType } from './../types'; +import { LuActionType } from '../types'; const ctx: Worker = self as any; const parse = (id: string, content: string) => { @@ -11,8 +11,8 @@ const parse = (id: string, content: string) => { }; ctx.onmessage = function (msg) { - const msgId = msg.data.id; - const { type, content, id, intentName, intent } = msg.data.payload; + const { id: msgId, type, payload } = msg.data; + const { content, id, intentName, intent } = payload; let result: any = null; try { switch (type) { diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts index 3a42c5b2ad..1f3a269b0a 100644 --- a/Composer/packages/client/src/utils/dialogUtil.ts +++ b/Composer/packages/client/src/utils/dialogUtil.ts @@ -13,7 +13,6 @@ import { import get from 'lodash/get'; import set from 'lodash/set'; import cloneDeep from 'lodash/cloneDeep'; -import { Expression } from 'adaptive-expressions'; import { IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; import { IComboBoxOption } from 'office-ui-fabric-react/lib/ComboBox'; import formatMessage from 'format-message'; @@ -338,16 +337,6 @@ export function sanitizeDialogData(dialogData: any) { return dialogData; } -export function isExpression(str: string): boolean { - try { - Expression.parse(str); - } catch (error) { - return false; - } - - return true; -} - export function getSelected(focused: string): string { if (!focused) return ''; return focused.split('.')[0]; diff --git a/Composer/packages/electron-server/src/electronWindow.ts b/Composer/packages/electron-server/src/electronWindow.ts index 673680936c..2d11e6b9d2 100644 --- a/Composer/packages/electron-server/src/electronWindow.ts +++ b/Composer/packages/electron-server/src/electronWindow.ts @@ -33,6 +33,7 @@ export default class ElectronWindow { width: adjustedWidth, height: adjustedHeight, webPreferences: { + nodeIntegrationInWorker: false, nodeIntegration: false, preload: join(__dirname, 'preload.js'), }, diff --git a/Composer/packages/lib/indexers/package.json b/Composer/packages/lib/indexers/package.json index 60a9886827..77e33f0fc6 100644 --- a/Composer/packages/lib/indexers/package.json +++ b/Composer/packages/lib/indexers/package.json @@ -29,8 +29,8 @@ "dependencies": { "@bfc/shared": "*", "@microsoft/bf-lu": "4.9.1", - "adaptive-expressions": "^4.10.0-preview-133287", - "botbuilder-lg": "^4.10.0-preview-133287", + "adaptive-expressions": "4.10.0-preview-135858", + "botbuilder-lg": "4.10.0-preview-135858", "lodash": "^4.17.15" } } diff --git a/Composer/packages/server/src/server.ts b/Composer/packages/server/src/server.ts index 12ffd6ece0..99e75eb65d 100644 --- a/Composer/packages/server/src/server.ts +++ b/Composer/packages/server/src/server.ts @@ -139,13 +139,13 @@ export async function start(pluginDir?: string): Promise { perMessageDeflate: false, }); - const { lgImportResolver, luImportResolver, staticMemoryResolver } = BotProjectService; + const { getLgResources, luImportResolver, staticMemoryResolver } = BotProjectService; function launchLanguageServer(socket: rpc.IWebSocket) { const reader = new rpc.WebSocketMessageReader(socket); const writer = new rpc.WebSocketMessageWriter(socket); const connection: IConnection = createConnection(reader, writer); - const server = new LGServer(connection, lgImportResolver, staticMemoryResolver); + const server = new LGServer(connection, getLgResources, staticMemoryResolver); server.start(); } diff --git a/Composer/packages/server/src/services/project.ts b/Composer/packages/server/src/services/project.ts index 3549c5fd78..1439777a07 100644 --- a/Composer/packages/server/src/services/project.ts +++ b/Composer/packages/server/src/services/project.ts @@ -35,19 +35,18 @@ export class BotProjectService { } } - public static lgImportResolver(source: string, id: string, projectId: string): ResolverResource { + public static getLgResources(projectId?: string): ResolverResource[] { BotProjectService.initialize(); const project = BotProjectService.getIndexedProjectById(projectId); if (!project) throw new Error('project not found'); - const resource = project.files.reduce((result: ResolverResource[], file) => { + const resources = project.files.reduce((result: ResolverResource[], file) => { const { name, content } = file; if (name.endsWith('.lg')) { result.push({ id: Path.basename(name, '.lg'), content }); } return result; }, []); - const resolver = importResolverGenerator(resource, '.lg'); - return resolver(source, id); + return resources; } public static luImportResolver(source: string, id: string, projectId: string): ResolverResource { diff --git a/Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts b/Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts index b5b2a7b66e..3daf75dad6 100644 --- a/Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts +++ b/Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts @@ -57,6 +57,14 @@ function jsonEscape(str) { const content = jsonEscape(lgFile); describe('LG LSP server test', () => { + const oldEnv = process.env; + beforeAll(() => { + process.env.NODE_ENV = 'test'; + }); + + afterAll(() => { + process.env = oldEnv; + }); const server = startServer(); beforeAll(async () => { await new Promise((resolve) => { diff --git a/Composer/packages/tools/language-servers/language-generation/__tests__/helpers/server.ts b/Composer/packages/tools/language-servers/language-generation/__tests__/helpers/server.ts index a04592b9b2..f9d2bd9ffa 100644 --- a/Composer/packages/tools/language-servers/language-generation/__tests__/helpers/server.ts +++ b/Composer/packages/tools/language-servers/language-generation/__tests__/helpers/server.ts @@ -1,7 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import fs from 'fs'; -import path from 'path'; import * as http from 'http'; import * as url from 'url'; import * as net from 'net'; @@ -13,16 +11,6 @@ import express from 'express'; import { LGServer } from '../../src'; -// eslint-disable-next-line security/detect-non-literal-fs-filename -const lgFile = fs.readFileSync(path.join(__dirname, '../mocks/greeting.lg'), 'utf-8'); - -const lgImportResolver = (_source, id) => { - return { - id, - content: lgFile, - }; -}; - const memoryResolver = () => { return ['this.value', 'this.turnCount']; }; @@ -59,7 +47,7 @@ function launchLanguageServer(socket: rpc.IWebSocket) { const reader = new rpc.WebSocketMessageReader(socket); const writer = new rpc.WebSocketMessageWriter(socket); const connection: IConnection = createConnection(reader, writer); - return new LGServer(connection, lgImportResolver, memoryResolver); + return new LGServer(connection, (projectId?: string) => [], memoryResolver); } export function startServer() { diff --git a/Composer/packages/tools/language-servers/language-generation/demo/src/server.ts b/Composer/packages/tools/language-servers/language-generation/demo/src/server.ts index 1e119654bc..ddd3048cfd 100644 --- a/Composer/packages/tools/language-servers/language-generation/demo/src/server.ts +++ b/Composer/packages/tools/language-servers/language-generation/demo/src/server.ts @@ -28,12 +28,12 @@ function launchLanguageServer(socket: rpc.IWebSocket) { const reader = new rpc.WebSocketMessageReader(socket); const writer = new rpc.WebSocketMessageWriter(socket); const connection: IConnection = createConnection(reader, writer); - const server = new LGServer(connection); + const server = new LGServer(connection, (projectId?: string) => []); server.start(); return server; } -attachLSPServer(wss, server, '/lg-language-server', webSocket => { +attachLSPServer(wss, server, '/lg-language-server', (webSocket) => { if (webSocket.readyState === webSocket.OPEN) { launchLanguageServer(webSocket); } else { diff --git a/Composer/packages/tools/language-servers/language-generation/package.json b/Composer/packages/tools/language-servers/language-generation/package.json index c2142ec5d5..f8eaa99266 100644 --- a/Composer/packages/tools/language-servers/language-generation/package.json +++ b/Composer/packages/tools/language-servers/language-generation/package.json @@ -8,7 +8,7 @@ "build:demo": "cd demo && tsc --build tsconfig.json", "prepublishOnly": "npm run build", "clean": "rimraf lib demo/dist", - "start": "cd demo && ts-node ./src/server.ts", + "start": "cd demo && cross-env NODE_ENV=test ts-node ./src/server.ts", "test": "jest", "lint": "eslint --quiet ./src ./__tests__", "lint:fix": "yarn lint --fix", @@ -16,12 +16,13 @@ }, "dependencies": { "@bfc/indexers": "*", - "botbuilder-lg": "^4.10.0-preview-133287", + "botbuilder-lg": "4.10.0-preview-135858", "vscode-languageserver": "^5.3.0-next" }, "devDependencies": { "@bfc/test-utils": "*", "@types/node": "^12.0.4", + "cross-env": "^6.0.3", "express": "^4.17.1", "rimraf": "^2.6.3", "ts-node": "^8.4.1", diff --git a/Composer/packages/tools/language-servers/language-generation/src/LGServer.ts b/Composer/packages/tools/language-servers/language-generation/src/LGServer.ts index 2a0e1ca584..9f9ccad03f 100644 --- a/Composer/packages/tools/language-servers/language-generation/src/LGServer.ts +++ b/Composer/packages/tools/language-servers/language-generation/src/LGServer.ts @@ -16,9 +16,9 @@ import { import { TextDocumentPositionParams, DocumentOnTypeFormattingParams } from 'vscode-languageserver-protocol'; import get from 'lodash/get'; import { filterTemplateDiagnostics, isValid } from '@bfc/indexers'; -import { MemoryResolver } from '@bfc/shared'; -import { ImportResolverDelegate, Templates } from 'botbuilder-lg'; +import { MemoryResolver, ResolverResource, LgTemplate } from '@bfc/shared'; +import { LgParser } from './lgParser'; import { buildInfunctionsMap } from './builtinFunctionsMap'; import { getRangeAtPosition, @@ -28,7 +28,6 @@ import { generageDiagnostic, LGOption, LGCursorState, - updateTemplate, cardTypes, cardPropDict, cardPropPossibleValueType, @@ -45,17 +44,11 @@ export class LGServer { protected readonly pendingValidationRequests = new Map(); protected LGDocuments: LGDocument[] = []; private memoryVariables: Record = {}; + private _lgParser = new LgParser(); constructor( protected readonly connection: IConnection, - protected readonly importResolver?: ( - source: string, - resourceId: string, - projectId: string - ) => { - content: string; - id: string; - }, + protected readonly getLgResources: (projectId?: string) => ResolverResource[], protected readonly memoryResolver?: MemoryResolver ) { this.documents.listen(this.connection); @@ -88,8 +81,8 @@ export class LGServer { }, }; }); - this.connection.onCompletion((params) => this.completion(params)); - this.connection.onHover((params) => this.hover(params)); + this.connection.onCompletion(async (params) => await this.completion(params)); + this.connection.onHover(async (params) => await this.hover(params)); this.connection.onDocumentOnTypeFormatting((docTypingParams) => this.docTypeFormat(docTypingParams)); this.connection.onRequest((method, params) => { @@ -148,16 +141,16 @@ export class LGServer { }); } - protected validateLgOption(document: TextDocument, lgOption?: LGOption) { + protected async validateLgOption(document: TextDocument, lgOption?: LGOption) { if (!lgOption) return; const diagnostics: string[] = []; - if (!this.importResolver) { - diagnostics.push('[Error lgOption] importResolver is required but not exist.'); + if (!this.getLgResources) { + diagnostics.push('[Error lgOption] getLgResources is required but not exist.'); } else { const { fileId, templateId } = lgOption; - const lgFile = this.getLGDocument(document)?.index(); + const lgFile = await this.getLGDocument(document)?.index(); if (!lgFile) { diagnostics.push(`[Error lgOption] File ${fileId}.lg do not exist`); } else if (templateId) { @@ -173,60 +166,33 @@ export class LGServer { ); } - protected getImportResolver(document: TextDocument) { - const editorContent = document.getText(); - const internalImportResolver = () => { - return { - id: document.uri, - content: editorContent, - }; - }; - const { fileId, templateId, projectId } = this.getLGDocument(document) || {}; - - if (this.importResolver && fileId && projectId) { - const resolver = this.importResolver; - return (source: string, id: string) => { - const lgFile = resolver(source, id, projectId); - if (!lgFile) { - this.sendDiagnostics(document, [ - generageDiagnostic(`lg file: ${fileId}.lg not exist on server`, DiagnosticSeverity.Error, document), - ]); - } - let { content } = lgFile; - /** - * source is . means use as file resolver, not import resolver - * if inline editor, server file write may have delay than webSocket updated LSP server - * so here build the full content from server file content and editor content - */ - if (source === '.' && templateId) { - content = updateTemplate(lgFile.content, templateId, editorContent); - } - return { id, content }; - }; - } - - return internalImportResolver; - } - protected addLGDocument(document: TextDocument, lgOption?: LGOption) { const { uri } = document; const { fileId, templateId, projectId } = lgOption || {}; - const index = () => { - const importResolver: ImportResolverDelegate = this.getImportResolver(document); + const index = async () => { let content: string = document.getText(); // if inline mode, composite local with server resolved file. - if (this.importResolver && fileId && templateId) { + if (this.getLgResources && fileId && templateId) { try { - content = importResolver('.', `${fileId}.lg`).content; + const resources = this.getLgResources(projectId); + content = resources.find((item) => item.id === fileId)?.content || content; } catch (error) { // ignore if file not exist } } const id = fileId || uri; - const { allTemplates, diagnostics } = Templates.parseText(content, id, importResolver); + let templates: LgTemplate[] = []; + let diagnostics: any[] = []; + try { + const payload = await this._lgParser.parseText(content, id, this.getLgResources(projectId)); + templates = payload.templates; + diagnostics = payload.diagnostics; + } catch (error) { + diagnostics.push(generageDiagnostic(error.message, DiagnosticSeverity.Error, document)); + } - return { templates: allTemplates, diagnostics }; + return { templates, diagnostics }; }; const lgDocument: LGDocument = { uri, @@ -242,12 +208,12 @@ export class LGServer { return this.LGDocuments.find(({ uri }) => uri === document.uri); } - protected hover(params: TextDocumentPositionParams): Thenable { + protected async hover(params: TextDocumentPositionParams): Promise> { const document = this.documents.get(params.textDocument.uri); if (!document) { return Promise.resolve(null); } - const lgFile = this.getLGDocument(document)?.index(); + const lgFile = await this.getLGDocument(document)?.index(); if (!lgFile) { return Promise.resolve(null); } @@ -511,7 +477,7 @@ export class LGServer { return completionList; } - protected completion(params: TextDocumentPositionParams): Thenable { + protected async completion(params: TextDocumentPositionParams): Promise> { const document = this.documents.get(params.textDocument.uri); if (!document) { return Promise.resolve(null); @@ -521,7 +487,7 @@ export class LGServer { const wordAtCurRange = document.getText(range); const endWithDot = wordAtCurRange.endsWith('.'); const lgDoc = this.getLGDocument(document); - const lgFile = lgDoc?.index(); + const lgFile = await lgDoc?.index(); const templateId = lgDoc?.templateId; const lines = document.getText(range).split('\n'); if (!lgFile) { @@ -722,9 +688,9 @@ export class LGServer { this.cleanPendingValidation(document); this.pendingValidationRequests.set( document.uri, - setTimeout(() => { + setTimeout(async () => { this.pendingValidationRequests.delete(document.uri); - this.doValidate(document); + await this.doValidate(document); }) ); } @@ -737,14 +703,14 @@ export class LGServer { } } - protected doValidate(document: TextDocument): void { + protected async doValidate(document: TextDocument): Promise { const text = document.getText(); const lgDoc = this.getLGDocument(document); if (!lgDoc) { return; } - const { fileId, templateId, uri } = lgDoc; - const lgFile = lgDoc.index(); + const { fileId, templateId, uri, projectId } = lgDoc; + const lgFile = await lgDoc.index(); if (!lgFile) { return; } @@ -778,7 +744,13 @@ export class LGServer { this.sendDiagnostics(document, lspDiagnostics); return; } - const lgDiagnostics = Templates.parseText(text, fileId || uri, this.getImportResolver(document)).diagnostics; + let lgDiagnostics: any[] = []; + try { + const payload = await this._lgParser.parseText(text, fileId || uri, this.getLgResources(projectId)); + lgDiagnostics = payload.diagnostics; + } catch (error) { + lgDiagnostics.push(generageDiagnostic(error.message, DiagnosticSeverity.Error, document)); + } const lspDiagnostics = convertDiagnostics(lgDiagnostics, document); this.sendDiagnostics(document, lspDiagnostics); } diff --git a/Composer/packages/tools/language-servers/language-generation/src/lgParser.ts b/Composer/packages/tools/language-servers/language-generation/src/lgParser.ts new file mode 100644 index 0000000000..9a2621802d --- /dev/null +++ b/Composer/packages/tools/language-servers/language-generation/src/lgParser.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { fork, ChildProcess } from 'child_process'; +import path from 'path'; + +import { ResolverResource } from '@bfc/shared'; +import uniqueId from 'lodash/uniqueId'; + +const isTest = process.env?.NODE_ENV === 'test'; + +export interface WorkerMsg { + id: string; + error?: any; + payload?: any; +} + +// Wrapper class +export class LgParser { + private worker: ChildProcess; + private resolves = {}; + private rejects = {}; + + constructor() { + const fileName = isTest ? 'lgWorker.ts' : 'lgWorker.js'; + const execArgv = isTest ? ['-r', 'ts-node/register'] : []; + const workerScriptPath = path.join(__dirname, fileName); + this.worker = fork(workerScriptPath, [], { execArgv }); + this.worker.on('message', this.handleMsg.bind(this)); + } + + public async parseText(content: string, id: string, resources: ResolverResource[]): Promise { + const msgId = uniqueId(); + const msg = { id: msgId, payload: { content, id, resources } }; + return new Promise((resolve, reject) => { + this.resolves[msgId] = resolve; + this.rejects[msgId] = reject; + this.worker.send(msg); + }); + } + + // Handle incoming calculation result + public handleMsg(msg: WorkerMsg) { + const { id, error, payload } = msg; + if (error) { + const reject = this.rejects[id]; + if (reject) reject(error); + } else { + const resolve = this.resolves[id]; + if (resolve) resolve(payload); + } + + // purge used callbacks + delete this.resolves[id]; + delete this.rejects[id]; + } +} diff --git a/Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts b/Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts new file mode 100644 index 0000000000..4cf26af4e2 --- /dev/null +++ b/Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Templates, Diagnostic } from 'botbuilder-lg'; +import { importResolverGenerator } from '@bfc/shared'; + +import { WorkerMsg } from './lgParser'; + +function createDiagnostic(diagnostic: Diagnostic) { + const { code, range, severity, source, message } = diagnostic; + const { start, end } = range; + return { + code, + range: { + start: { line: start.line, character: start.character }, + end: { line: end.line, character: end.character }, + }, + severity, + source, + message, + }; +} + +process.on('message', (message: WorkerMsg) => { + const { content, id, resources } = message.payload; + let templates: any[] = []; + let diagnostics: any[] = []; + try { + const resolver = importResolverGenerator(resources, '.lg'); + const { allTemplates, allDiagnostics } = Templates.parseText(content, id, resolver); + templates = allTemplates.map((item) => ({ name: item.name, parameters: item.parameters, body: item.body })); + diagnostics = allDiagnostics.map((item) => createDiagnostic(item)); + process.send?.({ id: message.id, payload: { templates, diagnostics } }); + } catch (error) { + process.send?.({ id: message.id, error }); + } +}); diff --git a/Composer/packages/tools/language-servers/language-generation/src/utils.ts b/Composer/packages/tools/language-servers/language-generation/src/utils.ts index 083c3fe3b7..74abc839b7 100644 --- a/Composer/packages/tools/language-servers/language-generation/src/utils.ts +++ b/Composer/packages/tools/language-servers/language-generation/src/utils.ts @@ -36,7 +36,7 @@ export interface LGDocument { projectId?: string; fileId?: string; templateId?: string; - index: () => LgParsed; + index: () => Promise; } export interface LGParsedResource { diff --git a/Composer/packages/ui-plugins/lg/src/LgField.tsx b/Composer/packages/ui-plugins/lg/src/LgField.tsx index 0a07b36b08..93e06deb77 100644 --- a/Composer/packages/ui-plugins/lg/src/LgField.tsx +++ b/Composer/packages/ui-plugins/lg/src/LgField.tsx @@ -49,7 +49,7 @@ const LgField: React.FC> = (props) => { const updateLgTemplate = useCallback( (body: string) => { - shellApi.updateLgTemplate(lgFileId, lgName, body).catch(() => {}); + shellApi.updateLgTemplate(lgFileId, lgName, body); }, [lgName, lgFileId] ); diff --git a/Composer/yarn.lock b/Composer/yarn.lock index b207ba091d..6bf708f97b 100644 --- a/Composer/yarn.lock +++ b/Composer/yarn.lock @@ -4399,21 +4399,6 @@ adal-angular@^1.0.17: resolved "https://registry.yarnpkg.com/adal-angular/-/adal-angular-1.0.17.tgz#6e936e0e41f91d3b2a88e7ffca9c2f6f6f562cc4" integrity sha1-bpNuDkH5HTsqiOf/ypwvb29WLMQ= -adal-node@^0.1.28: - version "0.1.28" - resolved "https://registry.yarnpkg.com/adal-node/-/adal-node-0.1.28.tgz#468c4bb3ebbd96b1270669f4b9cba4e0065ea485" - integrity sha1-RoxLs+u9lrEnBmn0ucuk4AZepIU= - dependencies: - "@types/node" "^8.0.47" - async ">=0.6.0" - date-utils "*" - jws "3.x.x" - request ">= 2.52.0" - underscore ">= 1.3.1" - uuid "^3.1.0" - xmldom ">= 0.1.x" - xpath.js "~1.1.0" - adal-node@^0.2.1: version "0.2.1" resolved "https://registry.npmjs.org/adal-node/-/adal-node-0.2.1.tgz#19e401bd579977448c1a77ce0e5b4c9accdc334e" @@ -4429,10 +4414,9 @@ adal-node@^0.2.1: xmldom ">= 0.1.x" xpath.js "~1.1.0" -adaptive-expressions@4.10.0-preview-133287, adaptive-expressions@^4.10.0-preview-133287: - version "4.10.0-preview-133287" - resolved "https://botbuilder.myget.org/F/botbuilder-v4-js-daily/npm/adaptive-expressions/-/adaptive-expressions-4.10.0-preview-133287.tgz#1a763d0b48bc639cf130f55e32e4c87cd6bfb255" - integrity sha1-GnY9C0i8Y5zxMPVeMuTIfNa/slU= +adaptive-expressions@4.10.0-preview-135858: + version "4.10.0-preview-135858" + resolved "https://botbuilder.myget.org/F/botbuilder-v4-js-daily/npm/adaptive-expressions/-/adaptive-expressions-4.10.0-preview-135858.tgz#b48f2849825cdb0b0876b7b8be001fbd7a171d14" dependencies: "@microsoft/recognizers-text-data-types-timex-expression" "1.1.4" "@types/atob-lite" "^2.0.0" @@ -5532,12 +5516,11 @@ boolean@^3.0.0: resolved "https://registry.yarnpkg.com/boolean/-/boolean-3.0.1.tgz#35ecf2b4a2ee191b0b44986f14eb5f052a5cbb4f" integrity sha512-HRZPIjPcbwAVQvOTxR4YE3o8Xs98NqbbL1iEZDCz7CL8ql0Lt5iOyJFxfnAB0oFs8Oh02F/lLlg30Mexv46LjA== -botbuilder-lg@^4.10.0-preview-133287: - version "4.10.0-preview-133287" - resolved "https://botbuilder.myget.org/F/botbuilder-v4-js-daily/npm/botbuilder-lg/-/botbuilder-lg-4.10.0-preview-133287.tgz#0f07eaf6e202146bb177c980a1ca8fce23751473" - integrity sha1-Dwfq9uICFGuxd8mAocqPziN1FHM= +botbuilder-lg@4.10.0-preview-135858: + version "4.10.0-preview-135858" + resolved "https://botbuilder.myget.org/F/botbuilder-v4-js-daily/npm/botbuilder-lg/-/botbuilder-lg-4.10.0-preview-135858.tgz#b0a96755d854ec17830b52a8f89beca31dd73b3f" dependencies: - adaptive-expressions "4.10.0-preview-133287" + adaptive-expressions "4.10.0-preview-135858" antlr4ts "0.5.0-alpha.3" lodash "^4.17.11" path "^0.12.7"