Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve the user experience about lg editor #3337

Merged
merged 31 commits into from
Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2e5be0a
move lg api into worker
a-b-r-o-w-n Jun 4, 2020
095631a
output worker files with readable names
a-b-r-o-w-n Jun 4, 2020
cc78eb9
update the version of botbuild-lg
lei9444 Jun 5, 2020
173b91c
Merge branch 'master' into lgperf
lei9444 Jun 5, 2020
837eaa0
remove promise in parse event
a-b-r-o-w-n Jun 5, 2020
4cbdb2a
refacotr lg worker to catch all errors
a-b-r-o-w-n Jun 5, 2020
db22978
Merge branch 'master' into lgperf
lei9444 Jun 8, 2020
402279d
add worker for lg lsp
lei9444 Jun 10, 2020
0eab492
Merge branch 'master' of https://github.com/microsoft/BotFramework-Co…
lei9444 Jun 10, 2020
16f9779
Merge branch 'master' into lgperf
a-b-r-o-w-n Jun 10, 2020
59a6739
add a null check when rejecting
a-b-r-o-w-n Jun 10, 2020
561d488
dissallow node integration in worker threads
a-b-r-o-w-n Jun 10, 2020
c0342ff
remove request_light
a-b-r-o-w-n Jun 10, 2020
a2c1169
update worker file naming
a-b-r-o-w-n Jun 10, 2020
9bb6ebc
Merge branch 'master' into lgperf
cwhitten Jun 10, 2020
e8c290f
fix unit test
lei9444 Jun 11, 2020
018d051
Merge branch 'master' into lgperf
lei9444 Jun 11, 2020
7c23a4a
set asar = false
lei9444 Jun 11, 2020
dd56752
Merge branch 'lgperf' of https://github.com/lei9444/BotFramework-Comp…
lei9444 Jun 11, 2020
ccf1b65
add force exit for jest
lei9444 Jun 11, 2020
b9f2c12
move node_module to unpack folder
lei9444 Jun 12, 2020
7f3b3e5
Merge branch 'master' into lgperf
lei9444 Jun 12, 2020
5df881b
Swapped out server side worker with child process.
tonyanziano Jun 16, 2020
1e54b47
Merge branch 'master' into lgperf
lei9444 Jun 17, 2020
16558cd
fix unit tests in lg lsp
lei9444 Jun 17, 2020
b025ada
clean the code
lei9444 Jun 17, 2020
4b59439
Merge branch 'master' of https://github.com/microsoft/BotFramework-Co…
lei9444 Jun 17, 2020
576a3af
only support test env in lg lsp
lei9444 Jun 17, 2020
9dcd837
fix some comments
lei9444 Jun 17, 2020
b377257
update the dev dependency
lei9444 Jun 17, 2020
6fc46a5
Merge branch 'master' into lgperf
lei9444 Jun 17, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Composer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to solve the tests somehow not exits propertly in CI envivonment like i listed above

"test:integration": "cypress run --browser edge",
"test:integration:start-server": "node scripts/e2e.js",
"test:integration:open": "cypress open",
Expand Down
2 changes: 1 addition & 1 deletion Composer/packages/client/__tests__/shell/lgApi.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
5 changes: 4 additions & 1 deletion Composer/packages/client/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions Composer/packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ const CodeEditor: React.FC<CodeEditorProps> = (props) => {

const _onChange = useCallback(
(value) => {
setContent(value);
if (!file) return;
if (inlineMode) {
if (!template) return;
Expand Down
11 changes: 3 additions & 8 deletions Composer/packages/client/src/shell/lgApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions Composer/packages/client/src/store/action/lg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 });
};
6 changes: 3 additions & 3 deletions Composer/packages/client/src/store/parsers/baseWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface WorkerMsg {
}

// Wrapper class
export class BaseWorker {
export class BaseWorker<ActionType> {
private worker: Worker;
private resolves = {};
private rejects = {};
Expand All @@ -22,9 +22,9 @@ export class BaseWorker {
this.worker.onmessage = this.handleMsg.bind(this);
}

public sendMsg<Payload>(payload: Payload) {
public sendMsg<Payload>(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;
Expand Down
6 changes: 3 additions & 3 deletions Composer/packages/client/src/store/parsers/indexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IndexerActionType> {
index(files: FileInfo, botName: string, schemas: any, locale: string) {
return this.sendMsg<IndexPayload>({ files, botName, schemas, locale });
return this.sendMsg<IndexPayload>(IndexerActionType.Index, { files, botName, schemas, locale });
}
}

Expand Down
40 changes: 36 additions & 4 deletions Composer/packages/client/src/store/parsers/lgWorker.ts
Original file line number Diff line number Diff line change
@@ -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<LgActionType> {
parse(targetId: string, content: string, lgFiles: LgFile[]) {
return this.sendMsg<LgPayload>({ targetId, content, lgFiles: lgFiles });
return this.sendMsg<LgParsePayload>(LgActionType.Parse, { targetId, content, lgFiles: lgFiles });
}

addTemplate(content: string, template: LgTemplate) {
return this.sendMsg<LgCreateTemplatePayload>(LgActionType.AddTemplate, { content, template });
}

updateTemplate(content: string, templateName: string, template: LgTemplate) {
return this.sendMsg<LgUpdateTemplatePayload>(LgActionType.UpdateTemplate, { content, templateName, template });
}

removeTemplate(content: string, templateName: string) {
return this.sendMsg<LgRemoveTemplatePayload>(LgActionType.RemoveTemplate, { content, templateName });
}

removeAllTemplates(content: string, templateNames: string[]) {
return this.sendMsg<LgRemoveAllTemplatesPayload>(LgActionType.RemoveAllTemplates, { content, templateNames });
}

copyTemplate(content: string, fromTemplateName: string, toTemplateName: string) {
return this.sendMsg<LgCopyTemplatePayload>(LgActionType.CopyTemplate, {
content,
fromTemplateName,
toTemplateName,
});
}
}

Expand Down
18 changes: 9 additions & 9 deletions Composer/packages/client/src/store/parsers/luWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ import { BaseWorker } from './baseWorker';
import { LuPayload, LuActionType } from './types';

// Wrapper class
class LuWorker extends BaseWorker {
class LuWorker extends BaseWorker<LuActionType> {
parse(id: string, content: string) {
const payload = { type: LuActionType.Parse, id, content };
return this.sendMsg<LuPayload>(payload);
const payload = { id, content };
return this.sendMsg<LuPayload>(LuActionType.Parse, payload);
}

addIntent(content: string, intent: LuIntentSection) {
const payload = { type: LuActionType.AddIntent, content, intent };
return this.sendMsg<LuPayload>(payload);
const payload = { content, intent };
return this.sendMsg<LuPayload>(LuActionType.AddIntent, payload);
}

updateIntent(content: string, intentName: string, intent?: LuIntentSection) {
const payload = { type: LuActionType.UpdateIntent, content, intentName, intent };
return this.sendMsg<LuPayload>(payload);
const payload = { content, intentName, intent };
return this.sendMsg<LuPayload>(LuActionType.UpdateIntent, payload);
}

removeIntent(content: string, intentName: string) {
const payload = { type: LuActionType.RemoveIntent, content, intentName };
return this.sendMsg<LuPayload>(payload);
const payload = { content, intentName };
return this.sendMsg<LuPayload>(LuActionType.RemoveIntent, payload);
}
}

Expand Down
45 changes: 42 additions & 3 deletions Composer/packages/client/src/store/parsers/types.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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',
}
Loading