ARTEMIS-5569 Include Monaco editor scripts locally #109
Conversation
|
Did you try removing and cloning the repository again, and then running yarn install? there were some major changes to project structure a while ago. Running yarn build produces this with your patch: |
|
OK, I managed to build |
|
Tested, I also see some errors reported by webpack: |
|
Well... I give up. My change compiles when running locally on development webpack configuration, but I'm unable to test it because I don't have working Artemis instance locally. I will leave this for a better day or a more knowledgeable JavaScript expert. |
|
Looks like it was fixed in Hawtio-Next https://github.com/hawtio/hawtio-next/pull/1187/files by I have no idea why it doesn't work in Artemis Console. The closest I get to an actual error is: |
|
@ViliusS That's a nice catch - but I don't think the code that you linked from hawtio makes it into artemis - we have our "own" SendMessage.tsx. I'm thinking webpack config in artemis might be problematic? |
|
I spend another 10 hours and reorganized dependencies according to https://github.com/hawtio/hawtio-next/blob/main/app/README.md . This fixed a build error.
|
|
I think I fixed it. Had to completely wipe out node_modules. The only minor issue I didn't found solution for is that Monaco worker files are put into root HTML folder and are not minimized. |
|
I can confirm that sending msg through frontend works (in yarn start mode at least, so development build) The resulting js bundle for patternfly is smaller, and the monaco js files ARE minified for production builds - for example, html.worker.js is 1074KB. main: 22.4MB, 50 files p.s. I have some issues on login screen on both branches though, but it seems the errors are not always the same ... 
|
|
p.s. node_modules folder in this branch is 477MB / 72k files instead of 649MB/195k files in main! |
I cannot test development build as I don't have Jolokia locally. At least the default 'developer' login works without issues. |
|
For what's it worth, I think the errors you are seeing on login screen might be solved in new |
|
I think it's best to bump @hawtio/react when new version of hawtio console comes out, and sync the version with the console. |
|
Hello - we've already released
I have a branch https://github.com/grgrzybek/activemq-artemis-console/commits/ggrzybek-console-fixes which I'm going to use as PR, but give me a day - I need to fix something elsewhere and I'll get back to this one. |
|
Generally, with my local 1.10.0 which was released last week to NPM, I have these fixes:
|
|
@grgrzybek thank you. Great to see it moving forward! Just make sure Monaco files are loaded locally instead of CDN. For some reason Monaco configuration from Also, while looking at JS bundles I found that Monaco (and probably some other) JS code is still duplicated in other.js and hawtio.js in Artemis Console (search for string "https://cdn."), but this code is never used. That's probably an issue for another day. |
|
These changes can perhaps be merged together - a quick look at differences in yarn.lock file tells me that there's a lot more changes in vilius' branch (and like mentioned, sending messages works + bundles/node_modules are smaller, so it's definitely an improvement) |
|
I'll definitely handle this CDN issue for Monaco - we had this in Hawtio, but I don't remember the details. Let me finish with deadlocks in Pax Web first. |
|
And I'll of course use the work from @ViliusS! |
|
I've verified the change. Without: import * as monacoEditor from 'monaco-editor'
import { loader } from '@monaco-editor/react'
loader.config({ monaco: monacoEditor, paths: {} })I see (when opening SendMessage dialog): When this code is used, I see: So there's still something missing. |
|
The stack leading to first CDN load is: |
|
BTW, I'm working on merged PRs (mine and @ViliusS'): |
|
@grgrzybek make sure you nuke node_modules before testing final solution. I had the same issue that JS chunks were incorrectly generated because node_modules where populated with dependencies from previous runs. After I did dependency refactoring, deleted node_modules and restested it then was generated correctly. Also, not sure how webpack-dev environment is loaded, but even on production bundle I still saw some references to CDN in the code. They were just never triggered. Before my changes there were 4-5 places mentioned in the various JS chunks regarding CDN. After changes it went down to 2 (which never triggers). |
|
Actually it's not about rm-rfing We have (here and in Hawtio too) two bundlers:
The point is to not let duplicate code to sneak in anywhere. For this purpose I check which means that
modules among dependencies, so For webpack it's a bit more complicated, as we have chunks etc. But I simply open the files in Gvim and do Mind that this is the final bundle and it should contain everything - including |
|
By grepping the output of
as dependencies in the package. Checking now. |
|
And now it works: |
|
As I'm not familiar how exactly JS dev infrastructure works I spent hours trying to find why tsup generates large JS file and why there are code duplicates in the final. Now, with your explanation, everything makes sense. It's just sad that neither tsup, nor webpack, nor yarn documentation is really useful for newcomers like me. Anyway, thank you again and we will be waiting for the final release. |
|
Now with
Doing some statistics ("module" is a single JavaScript file from all used NPM Packages):
Now I'll check the duplicates (there shouldn't be any) and icons/styles/tokens optimization |
|
@ViliusS I found ChatGPT quite useful explaining how webpack, tsup and bundlers work. it helped me (but lead me to some dead ends too) with peer/dev dependencies etc. I can say one thing - Java+Maven is a beautiful combination comparing to what we see in JavaScript+bundlers. |
|
Looks like the released |
|
In prod mode (before icons optimization) there's 15.5MB of JavaScript: |
|
Strange that you still have |
|
With https://issues.apache.org/jira/browse/ARTEMIS-5600:
|
|
Hmm, indeed... when I build it I can see this: this |
|
I've just pushed #110 which combines my changes and @ViliusS changes... I know it's a big one, but this is special situation (big Hawtio upgrade). @ViliusS may I ask you to have a look at this one? I always prefer separate commits over squashed ones for clarity and future investigation ("why it was done like this?") |
|
@grgrzybek, the production build from your branch looks great. Monaco editor is loaded locally and SendMessage works. PatternFly chunk is much smaller, which is cool. I don't see I still see some Monaco loader code in other.js chunk (search for string "https://cdn."), but that probably is never loaded. I will close this PR in favour of yours, and move new discussion there. |
this is part of |
This should fix an issue reported on the mailing list, where Monaco Editor resources are loaded from CDN and thus blocked by CSP policy.
It uses the same mechanics as described in https://github.com/suren-atoyan/monaco-react?tab=readme-ov-file#use-monaco-editor-as-an-npm-package
I'm leaving this as draft as I could not test it fully
due to this build failure (it was the same before my changes):
Module not found: Error: Can't resolve 'artemis-console-plugin/styles' in 'C:\Users\myuser\git\activemq-artemis-console\artemis-console-extension\artemis-extension\app\src'Looks like
artemis-console-plugin/stylesis not public.