KOGITO-3195: Fork KIE Editors #494

tiagobento · 2021-04-30T17:09:07Z

We finally have a snapshot version of the new forked editors from kogito-editors-java. This PR is a huge milestone for Kogito Tooling, so it deserves a good description. I'll try to summarise everything included on this PR, as it's the result of a couple of month's work and we did many improvements that were not planned beforehand!

Update Editors to 8.0.0-SNAPSHOT-20210430.130828
Remove editorContext from the init method of the Envelopes. This context is now part of the Init args we pass when opening Editors.
With the Ace editor removed from the forked Editors, we needed a new way to handle invalid files, so we added Monaco to the Online and Desktop channels, so there are some new UIs there.
Made Chrome Extension's token text input be closed by default.
Manually tree-shook @patternfly/* dependencies. When we're using ESM, this will be done automatically, but for now, we have to be extra-careful with our imports to not bring the entire PatternFly parts into our bundles.
Made the Editor interface accept custom APIs. This will be very useful when we need to provide different APIs for some of our Editors. Note that the Channels are still using the Editors with the default Editor API.
Added a custom action upon undo/redoing stuff on the StateControl class.
Removed CompositeEditorFactory as we don't want to Editors to be bundled in the same Envelope.
Improved CSS loading on the Online Editor. Please note that further improvements will be done on KOGITO-3560.

…tandalone

…tests

…fork2

…#25) * Everything working except for Standalone Editors * Standalone Editors working. * Cleanup * Fix tests * Reduce Editor Envelope size. Remove PMML Marshalelr from BPMN and SceSim envelopes. * Remove unnecessary loader

tomasdavidorg · 2021-05-04T09:18:05Z

@tiagobento The desktop, Chrome and online editor tests are random failures. I ran all the tests locally in Windows virtual machine and they passed but I tried to lower the number of processors in the virtual machine to 2 and in some of the runs I was able to see this screen with two loading popups what is suspicious:

I saw this only when the online editor server was started and cypress tests were executed (processor usage was on 100%). Manually (without Cypress) I was not able to reproduce it.

tiagobento · 2021-05-04T11:31:35Z

@tomasdavidorg Thanks for taking a look! Chrome has a feature called CPU throttling that might be able to help reproduce that issue, have you tried it?

tiagobento · 2021-05-04T11:41:21Z

@domhanak Yes, Standalone Editors and Desktop are ok locally on my macOS.

tomasdavidorg · 2021-05-04T12:53:41Z

@tomasdavidorg Thanks for taking a look! Chrome has a feature called CPU throttling that might be able to help reproduce that issue, have you tried it?

@tiagobento Thanks for the tip but I was still not able to reproduce it. With the The CPU throttling the loading popup is just there for a longer time and at the end the editor appears.

However I was able to reproduce it by opening 10 windows of online editor. On every I clicked "Try dmn sample" and 4 of them end up with the loading issue. In the console I found:

Uncaught (in promise) TypeError: Cannot read property 'languages' of undefined
    at uQk (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24045)
    at vQk.wQk [as VC] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24343)
    at xQk.EQk [as VC] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24343)
    at dXk.aXk [as vD] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24344)
    at EXp.GXp [as KK] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24377)
    at e8o (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:7944)
    at c8o (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:19103)
    at Function.t8o (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24372)
    at d (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:17236)

tomasdavidorg · 2021-05-04T13:07:32Z

I am able to reproduce one more issue. If I open 10 tabs with the online editor and in each I click "Create new decision model" I got this error in some of them:

:9001/#/editor/dmn:1 Uncaught (in promise) TypeError: Cannot read property 'iR' of undefined
    at dXk.UWk [as dD] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24344)
    at nXk.vXk [as dD] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24344)
    at FVk.GVk [as dD] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24344)
    at RVk.VVk [as dD] (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24344)
    at $wnd.GWTEditor.getContent (org.kie.workbench.common.dmn.showcase.DMNKogitoRuntimeWebapp-0.js:24012)
    at DmnEditorImpl.module.exports../src/common/GwtEditorWrapper.tsx.GwtEditorWrapper.getContent (index.js?7f42:351)
    at DmnEditorEnvelopeApiImpl.module.exports../src/envelope/KogitoEditorEnvelopeApiImpl.ts.KogitoEditorEnvelopeApiImpl.receive_contentRequest (index.js?a878:1140)
    at EnvelopeBusMessageManager.module.exports../src/common/EnvelopeBusMessageManager.ts.EnvelopeBusMessageManager.receive (index.js?69c4:345)
    at Object.receive (index.js?69c4:234)
    at EnvelopeBusController.module.exports../src/envelope/EnvelopeBusController.ts.EnvelopeBusController.receive (index.js?69c4:516)
    at eventListener (index.js?69c4:497)

tiagobento · 2021-05-04T13:48:35Z

@tomasdavidorg I can't reproduce those. Can you please give more details? Are you running it locally with yarn start on packages/online-editor? Are you using CPU throttling?

tomasdavidorg · 2021-05-05T05:52:13Z

@tiagobento I am able to reproduce it only on Windows 10 virtual machine (Virtual box). The machine has 2 CPUs, 7 GB RAM. Inside machine I start yarn run start from the online editor package. Then I open 10 tabs in Chrome browser with the online editor (http://localhost:9001/). I don't use CPU throttling.

To reproduce the issues with 2 loading popups, I click Try sample (DMN) in all the tabs.
To reproduce the issue with opening error, I click Create new decision model" in all the tabs.

file.mp4

manstis

The Monaco editor for "standalone" breaks with this PR.

Can you also advise why the need to change imports? Unless you communicate the reasoning to all contributors the old style will creep back in and whatever issue the change fixes will return.

manstis · 2021-05-05T10:08:49Z

packages/pmml-editor/webpack.config.js

@@ -31,7 +30,6 @@ module.exports = [
      libraryTarget: "commonjs2",
    },
    externals: [nodeExternals({ modulesDir: "../../node_modules" })],
-    plugins: [new MonacoWebpackPlugin()],


Now you've removed the MonacoWebpackPlugin the "standalone" PMML editor no longer has support for:-

Syntax highlighting

Code complete

You may need to add a similar entry to this so that the standalone editor works correctly?

master autocomplete (ctrl-space)

master syntax highlighting

the_fork2 autocomplete missing (ctrl-space)

the_fork2 syntax highlighting missing

Hi Michael!

WebpackMonacoPlugin was just a fancy way of rebuilding the Monaco Editor. We're actually fancier than that as we have our own publication of Monaco under @kiegroup/monaco-editor 😎

Jokes aside, I can't reproduce it. See this VSIX I just generared.
vscode_extension_kogito_kie_editors_0.9.1.vsix.zip

The icons are missing like @jomarko reported, though.

@tiagobento The VSCode extension is not affected, only pmml-editor when run with yarn run start. This is the webapp we use use day to day development.

Oh, okay. Forgot about that! Good catch, I'll take a look.

Fixed in 1b458cf!

manstis · 2021-05-05T10:15:44Z

Hi @kelvah I've added you for review too.

jomarko · 2021-05-05T14:15:03Z

01

Some monaco editor issue (#494 (review)) is present also for DMN in VS code.

Once autocompletion suggestions are displayed for DMN literal expression, the icons seems to be not loaded.

VS Code

dmn.new (online editor)

tiagobento · 2021-05-05T15:00:05Z

Hi @jomarko thanks for reporting that. I really just don't wanna block this PR by this issue, if possible. Do you think we can create a separate Jira for this one? If so, we can use this Epic https://issues.redhat.com/browse/KOGITO-5039

EDIT:
Fixed in 6663ddc for this repo. And here kiegroup/kogito-editors-java#21 for kogito-editors-java.

Online Editor:
online-editor-pr-494.zip

VSIX:
vscode_extension_kogito_kie_editors_0.9.1.vsix.zip

tiagobento · 2021-05-05T15:06:45Z

@tomasdavidorg Can you report this one under https://issues.redhat.com/browse/KOGITO-5039?

jomarko

I can confirm the icons are now present id DMN visx.

I tested manually the DMN and SCESIM editors. (Created small model with ~5 nodes and 4 scenarios for it).
I didn't checked code
I didn't checked other platforms than VS Code.

tomasdavidorg · 2021-05-06T06:42:28Z

@tomasdavidorg Can you report this one under https://issues.redhat.com/browse/KOGITO-5039?

@tiagobento Reported: https://issues.redhat.com/browse/KOGITO-5068

manstis

Thank-you for the fix @tiagobento.

manstis · 2021-05-06T08:13:26Z

@tiagobento I have approved; but please be sure to communicate "...why the need to change imports? Unless you communicate the reasoning to all contributors the old style will creep back in and whatever issue the change fixes will return".

tiagobento · 2021-05-06T12:54:19Z

@manstis The change on imports is related to item 5 on the description. Basically by importing the classes directly from their most specific directory, we are only importing what we really need. That's called tree-shaking. This is done automatically when using ES Modules, but since we're still on CommonJS, we had to do this change. There's an "automatic" way of doing it, but it's quite "magical" so I preferred to do that manually.

On the upcoming weeks we'll be migrating from CommonJS to ESM, meaning that tree-shaking will be automatic and we will be able to import @patternfly/react-core as a whole without inflating our bundles unnecessarily. In the mean time, please use the most specific import path you can find.

tiagobento · 2021-05-06T13:01:59Z

@kelvah @domhanak @tomasdavidorg Already got approvals for QE and the PMML Editor team, can we proceed and merge this PR?

tomasdavidorg · 2021-05-06T13:17:13Z

@tiagobento Please wait for review by @LuboTerifaj. We agreed that @LuboTerifaj will do some more checks of the BPMN editor. Thanks.

kelvah

Hi @tiagobento, everything looks good for the pmml-editor. I left you a couple of questions. Thank you.

kelvah · 2021-05-06T14:59:51Z

package.json

    "@kogito-tooling/quarkus-runner-unpacked": "0.7.0-SNAPSHOT",
-    "@kogito-tooling/scesim-editor-unpacked": "7.53.0-Final",
+    "@kogito-tooling/scesim-editor-unpacked": "8.0.0-SNAPSHOT-20210430.130828",
+    "monaco-editor": "^0.23.0",


I am bit confused. Why did you add a monaco-editor dep here?
Maybe I'm wrong but isn't @kiegroup/monaco-editor acting like a wrapper for monaco? In that case I think it should replace it as a dependency

Good catch! monaco-editor@^0.23.0 is listed as a devDependency on @kiegroup/monaco-editor as we didn't want to enforce the use of the original package when using it. The problem is that the type definitions are all in the original package, so we need to download it here too. When releasing @kiegroup/monaco-editor@0.2.3 I didn't remember that, so we would need a new @kiegroup/monaco-editor release listing monaco-editor@^0.23.0 as a dependency instead of devDependency. If it's okay I can do it on a new PR.

It's okay for me 👍

kelvah · 2021-05-06T15:06:00Z

packages/pmml-editor/src/editor/PMMLEditor.tsx

@@ -31,7 +31,7 @@ import { Provider } from "react-redux";
 import mergeReducers from "combine-reducer";
 import { HistoryContext, HistoryService } from "./history";
 import { LandingPage } from "./components/LandingPage/templates";
-import { Page } from "@patternfly/react-core";
+import { Page } from "@patternfly/react-core/dist/js/components/Page";


I saw your comments about "manual" tree shacking. Did you give a try to what PF suggests (babel-plugin-transform-imports for our case I think)?
Is it the magical solution you mentioned that wasn't ideal?
Asking because the manual change looks quite invasive.

Hi! Yes. I'm not saying it isn't ideal, I just didn't thought I wanted a tool dealing with the imports. It's one more thing to take care of and on the upcoming weeks we'll have ESM as default anyways..

I understand. Thank you.

LuboTerifaj

Hi all,

I didn't find any issues in VSCode bpmn editor.

Thank you!

tomasdavidorg · 2021-05-07T10:39:35Z

Thanks @LuboTerifaj.

tiagobento and others added 30 commits February 25, 2021 17:07

Remove Ace editor

e398865

Add setContentError workflow to Chrome Extension

60e526e

Fix label

d0486c1

Add text editor to Desktop and small fixes on chrome-ext/vscode-ext/s…

cc0c187

…tandalone

Fix i18n on Desktop text editor

3ea8e18

Add text editor to Online and small fixes on desktop too

8958936

Fix undo/redo after setContent fix

b76c2ff

Fix CSS bug on alerts chrome-extension

46d4ac9

Adding setContentError to Standalone Editors

adc9fd1

Fix Standalone Editors README

98f7752

Use Monaco 2.2.0, remove CompositeEditorFactory and fix StateControl …

57eef97

…tests

Fix tests

9c94b82

Removing EditorContext in favour of EditorInitArgs

c4ea754

Reduce Editor Envelope bundle size

d7201ed

PatternFly tree shaking and monaco on standalone editors

8c2335f

Envelope is way smaller now.

0fedf47

Fix tests and Standalone Editors bundle

95426e6

Merge remote-tracking branch 'origin/master' into the-fork2

cb40af2

Externalize Monaco Editor on Online channel

f03609f

Reorganize code

1499da0

Merge remote-tracking branch 'origin/master' into the-fork2

ef94569

Updating PMML Editor to use simplified Monaco

39417ae

Oops. Fixing last commit

f6e1722

Exclude SceSim files from showing up during build logs

38013f8

Fix light font color on PMML Editor in Dark mode

cc1ea99

Tree-shake PMML Editor to reduce bundle size

858c3bd

Merge branch 'master' of github.com:kiegroup/kogito-tooling into the-…

a36ba50

…fork2

Fix setContent error on PMML Editor

8561b96

Fix KOGITO-4869, KOGITO-4874 and KOGITO-4875

bd7f317

manstis suggested changes May 5, 2021

View reviewed changes

manstis requested a review from kelvah May 5, 2021 10:15

tiagobento added 2 commits May 5, 2021 14:46

Upgrade @kiegroup/monaco-editor with codeicon font fix

6663ddc

Fix Monaco on PMML Editor dev webapp

1b458cf

jomarko approved these changes May 6, 2021

View reviewed changes

manstis approved these changes May 6, 2021

View reviewed changes

tomasdavidorg requested a review from LuboTerifaj May 6, 2021 13:15

tomasdavidorg approved these changes May 6, 2021

View reviewed changes

kelvah approved these changes May 6, 2021

View reviewed changes

LuboTerifaj approved these changes May 7, 2021

View reviewed changes

domhanak approved these changes May 7, 2021

View reviewed changes

tiagobento merged commit 0b15f72 into apache:master May 7, 2021

domhanak mentioned this pull request May 7, 2021

Upgrade version of vscode-extension-tester to 4.0.3 #495

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KOGITO-3195: Fork KIE Editors #494

KOGITO-3195: Fork KIE Editors #494

tiagobento commented Apr 30, 2021

tomasdavidorg commented May 4, 2021 •

edited

Loading

tiagobento commented May 4, 2021

tiagobento commented May 4, 2021

tomasdavidorg commented May 4, 2021 •

edited

Loading

tomasdavidorg commented May 4, 2021

tiagobento commented May 4, 2021

tomasdavidorg commented May 5, 2021 •

edited

Loading

manstis left a comment

manstis May 5, 2021

manstis May 5, 2021

tiagobento May 5, 2021

manstis May 5, 2021

tiagobento May 5, 2021

tiagobento May 5, 2021

manstis commented May 5, 2021

jomarko commented May 5, 2021

tiagobento commented May 5, 2021 •

edited

Loading

tiagobento commented May 5, 2021

jomarko left a comment

tomasdavidorg commented May 6, 2021 •

edited

Loading

manstis left a comment

manstis commented May 6, 2021

tiagobento commented May 6, 2021 •

edited

Loading

tiagobento commented May 6, 2021

tomasdavidorg commented May 6, 2021 •

edited

Loading

kelvah left a comment

kelvah May 6, 2021 •

edited

Loading

tiagobento May 6, 2021

kelvah May 6, 2021

kelvah May 6, 2021

tiagobento May 6, 2021

kelvah May 6, 2021

LuboTerifaj left a comment

tomasdavidorg commented May 7, 2021

KOGITO-3195: Fork KIE Editors #494

KOGITO-3195: Fork KIE Editors #494

Conversation

tiagobento commented Apr 30, 2021

tomasdavidorg commented May 4, 2021 • edited Loading

tiagobento commented May 4, 2021

tiagobento commented May 4, 2021

tomasdavidorg commented May 4, 2021 • edited Loading

tomasdavidorg commented May 4, 2021

tiagobento commented May 4, 2021

tomasdavidorg commented May 5, 2021 • edited Loading

manstis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manstis commented May 5, 2021

jomarko commented May 5, 2021

01

tiagobento commented May 5, 2021 • edited Loading

tiagobento commented May 5, 2021

jomarko left a comment

Choose a reason for hiding this comment

tomasdavidorg commented May 6, 2021 • edited Loading

manstis left a comment

Choose a reason for hiding this comment

manstis commented May 6, 2021

tiagobento commented May 6, 2021 • edited Loading

tiagobento commented May 6, 2021

tomasdavidorg commented May 6, 2021 • edited Loading

kelvah left a comment

Choose a reason for hiding this comment

kelvah May 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuboTerifaj left a comment

Choose a reason for hiding this comment

tomasdavidorg commented May 7, 2021

tomasdavidorg commented May 4, 2021 •

edited

Loading

tomasdavidorg commented May 4, 2021 •

edited

Loading

tomasdavidorg commented May 5, 2021 •

edited

Loading

tiagobento commented May 5, 2021 •

edited

Loading

tomasdavidorg commented May 6, 2021 •

edited

Loading

tiagobento commented May 6, 2021 •

edited

Loading

tomasdavidorg commented May 6, 2021 •

edited

Loading

kelvah May 6, 2021 •

edited

Loading