fix: improve the user experience about lg editor #3337

lei9444 · 2020-06-05T07:04:26Z

Description

The lg parser takes a long time to parse a large file(20k lines will take 115s). And some lg template related functions are in UI thread. So the ui is blocked by the parser.

Now

move all parser function to web worker. complex calculation will not block the UI thread
update the botbuild-lg version. The parse time decreases from 115s to 10s (20k lines).
add worker for lg lsp to parse the lg file
But there is still an issue, the text in dialog will take a long time to show the update:

Task Item

refs #3295
refs #3314

Screenshots

coveralls · 2020-06-05T07:18:54Z

Coverage decreased (-0.04%) to 48.021% when pulling 6fc46a5 on lei9444:lgperf into 0bbab12 on microsoft:master.

coveralls · 2020-06-05T07:19:04Z

Coverage decreased (-0.09%) to 40.409% when pulling 173b91c on lei9444:lgperf into 069fe49 on microsoft:master.

…mposer into lgperf

Composer/packages/electron-server/src/electronWindow.ts

Composer/packages/tools/language-servers/language-generation/src/lgParser.ts

Composer/packages/tools/language-servers/language-generation/package.json

a-b-r-o-w-n

This looks good to me now.

boydc2014 · 2020-06-11T00:00:21Z

@lei9444 please fix the UT

boydc2014 · 2020-06-11T04:46:38Z

=============================== Coverage summary ===============================
Statements : 50.61% ( 4719/9325 )
Branches : 41.03% ( 1871/4560 )
Functions : 45.29% ( 1072/2367 )
Lines : 51.1% ( 4495/8797 )

Test Suites: 149 passed, 149 total
Tests: 575 passed, 575 total
Snapshots: 6 passed, 6 total
Time: 203.488 s
Ran all test suites in 10 projects.
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with --detectOpenHandles to troubleshoot this issue.
##[error]The operation was canceled.

I saw the UT passed, but somehow it get canceled.

…oser-1 into lgperf

Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts

…mposer into lgperf

tonyanziano

A few minor issues, but other than that it looks good to me. 👍

Composer/packages/tools/language-servers/language-generation/src/lgParser.ts

Composer/packages/tools/language-servers/language-generation/package.json

Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts

Composer/packages/tools/language-servers/language-generation/package.json

tonyanziano

Everything seems good now! Nice job

a-b-r-o-w-n · 2020-06-17T16:01:00Z

Composer/package.json

@@ -53,7 +53,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",


What does this do?

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

a-b-r-o-w-n · 2020-06-17T16:02:01Z

Composer/packages/tools/language-servers/language-generation/__tests__/LGServer.test.ts

+
+  afterEach(() => {
+    delete process.env.NODE_ENV;
+  });


Why isn't the node env already test when running tests? This doesn't seem right.

a-b-r-o-w-n · 2020-06-17T16:02:45Z

Composer/packages/tools/language-servers/language-generation/package.json

@@ -8,15 +8,15 @@
    "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 && set NODE_ENV=development&& ts-node ./src/server.ts",


This isn't cross platform friendly.

@a-b-r-o-w-n this got fixed already with cross-env

zhixzhan · 2020-08-10T03:00:20Z

Composer/packages/tools/language-servers/language-generation/src/LGServer.ts

        } catch (error) {
          // ignore if file not exist
        }
      }

      const id = fileId || uri;
-      const { allTemplates, diagnostics } = Templates.parseText(content, id, importResolver);


Here we still need allTemplates for intelligent suggestions (items in imported file)

* 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 <asbrown002@gmail.com> Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com> Co-authored-by: Tony Anziano <tonyanziano5@gmail.com>

a-b-r-o-w-n and others added 3 commits June 4, 2020 14:20

move lg api into worker

2e5be0a

output worker files with readable names

095631a

update the version of botbuild-lg

cc78eb9

lei9444 requested review from a-b-r-o-w-n, beyackle, boydc2014, cwhitten, srinaath and tonyanziano as code owners June 5, 2020 07:04

Merge branch 'master' into lgperf

173b91c

a-b-r-o-w-n and others added 5 commits June 5, 2020 08:42

remove promise in parse event

837eaa0

refacotr lg worker to catch all errors

4cbdb2a

Merge branch 'master' into lgperf

db22978

add worker for lg lsp

402279d

Merge branch 'master' of https://github.com/microsoft/BotFramework-Co…

0eab492

…mposer into lgperf

a-b-r-o-w-n requested changes Jun 10, 2020

View reviewed changes

a-b-r-o-w-n added 5 commits June 10, 2020 14:56

Merge branch 'master' into lgperf

16f9779

add a null check when rejecting

59a6739

dissallow node integration in worker threads

561d488

remove request_light

c0342ff

update worker file naming

a2c1169

a-b-r-o-w-n self-requested a review June 10, 2020 22:45

a-b-r-o-w-n previously approved these changes Jun 10, 2020

View reviewed changes

Merge branch 'master' into lgperf

9bb6ebc

fix unit test

e8c290f

lei9444 dismissed a-b-r-o-w-n’s stale review via e8c290f June 11, 2020 03:45

boydc2014 previously approved these changes Jun 11, 2020

View reviewed changes

lei9444 added 2 commits June 11, 2020 20:00

set asar = false

7c23a4a

Merge branch 'lgperf' of https://github.com/lei9444/BotFramework-Comp…

dd56752

…oser-1 into lgperf

lei9444 dismissed boydc2014’s stale review via dd56752 June 11, 2020 12:01

lei9444 added 3 commits June 11, 2020 20:25

add force exit for jest

ccf1b65

move node_module to unpack folder

b9f2c12

Merge branch 'master' into lgperf

7f3b3e5

hibrenda mentioned this pull request Jun 15, 2020

Language Generation Loading #3407

Closed

Swapped out server side worker with child process.

5df881b

tonyanziano reviewed Jun 16, 2020

View reviewed changes

Composer/packages/tools/language-servers/language-generation/src/lgWorker.ts Outdated Show resolved Hide resolved

lei9444 added 4 commits June 17, 2020 15:57

Merge branch 'master' into lgperf

1e54b47

fix unit tests in lg lsp

16558cd

clean the code

b025ada

Merge branch 'master' of https://github.com/microsoft/BotFramework-Co…

4b59439

…mposer into lgperf

tonyanziano suggested changes Jun 17, 2020

View reviewed changes

lei9444 added 2 commits June 18, 2020 01:02

only support test env in lg lsp

576a3af

fix some comments

9dcd837

tonyanziano reviewed Jun 17, 2020

View reviewed changes

Composer/packages/tools/language-servers/language-generation/package.json Show resolved Hide resolved

lei9444 added 2 commits June 18, 2020 01:53

update the dev dependency

b377257

Merge branch 'master' into lgperf

6fc46a5

tonyanziano approved these changes Jun 17, 2020

View reviewed changes

a-b-r-o-w-n reviewed Jun 17, 2020

View reviewed changes

cwhitten approved these changes Jun 17, 2020

View reviewed changes

cwhitten merged commit fd6b3c8 into microsoft:master Jun 17, 2020

hibrenda mentioned this pull request Jun 22, 2020

opening project is extremely slow now #3295

Closed

7 tasks

cwhitten mentioned this pull request Jul 8, 2020

release: 1.0.2 #3595

Merged

zhixzhan reviewed Aug 10, 2020

View reviewed changes

zhixzhan mentioned this pull request Aug 10, 2020

fix: lg editor validation error & intelligence #3816

Merged

lei9444 deleted the lgperf branch August 26, 2020 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: improve the user experience about lg editor #3337

fix: improve the user experience about lg editor #3337

lei9444 commented Jun 5, 2020 •

edited

Loading

coveralls commented Jun 5, 2020 •

edited

Loading

coveralls commented Jun 5, 2020

a-b-r-o-w-n left a comment

boydc2014 commented Jun 11, 2020

boydc2014 commented Jun 11, 2020

tonyanziano left a comment

tonyanziano left a comment

a-b-r-o-w-n Jun 17, 2020

boydc2014 Jun 17, 2020

a-b-r-o-w-n Jun 17, 2020

a-b-r-o-w-n Jun 17, 2020

tonyanziano Jun 17, 2020

zhixzhan Aug 10, 2020

fix: improve the user experience about lg editor #3337

fix: improve the user experience about lg editor #3337

Conversation

lei9444 commented Jun 5, 2020 • edited Loading

Description

Task Item

Screenshots

coveralls commented Jun 5, 2020 • edited Loading

coveralls commented Jun 5, 2020

a-b-r-o-w-n left a comment

Choose a reason for hiding this comment

boydc2014 commented Jun 11, 2020

boydc2014 commented Jun 11, 2020

=============================== Coverage summary =============================== Statements : 50.61% ( 4719/9325 ) Branches : 41.03% ( 1871/4560 ) Functions : 45.29% ( 1072/2367 ) Lines : 51.1% ( 4495/8797 )

tonyanziano left a comment

Choose a reason for hiding this comment

tonyanziano left a comment

Choose a reason for hiding this comment

a-b-r-o-w-n Jun 17, 2020

Choose a reason for hiding this comment

boydc2014 Jun 17, 2020

Choose a reason for hiding this comment

a-b-r-o-w-n Jun 17, 2020

Choose a reason for hiding this comment

a-b-r-o-w-n Jun 17, 2020

Choose a reason for hiding this comment

tonyanziano Jun 17, 2020

Choose a reason for hiding this comment

zhixzhan Aug 10, 2020

Choose a reason for hiding this comment

lei9444 commented Jun 5, 2020 •

edited

Loading

coveralls commented Jun 5, 2020 •

edited

Loading

=============================== Coverage summary ===============================
Statements : 50.61% ( 4719/9325 )
Branches : 41.03% ( 1871/4560 )
Functions : 45.29% ( 1072/2367 )
Lines : 51.1% ( 4495/8797 )