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

chore(deps): upgrade monaco-editor to 0.30 #11593

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Aug 16, 2023

Partially replaces #11475, but there is still more work to be done as written below

Motivation

Was asked to help upgrade deps, particularly ones that may have vulns since Snyk opened #11475

  • Though it doesn't actually mention any CVEs, but good to upgrade anyway (and I got a good part of the way in just investigating the issue)

Modifications

  • upgrade monaco-editor to 0.30

    • 0.30 requires monaco-editor-webpack-plugin 6, per the version matrix
    • 0.29.0 had a breaking change to renderIndentGuides; converted to the new option
      • true might not be necessary as it's the default, but guides is empty by default, so not 100% sure -- left it in for now to ensure the behavior remains the same after the upgrade
  • 0.30 is the latest version I could upgrade to without requiring significant build changes

    • 0.31+ uses newer JS syntax per the breaking changes
      • Per https://stackoverflow.com/a/70622999/3431180, this requires an updated parser, otherwise the Webpack build was getting similar errors as in the SO question
        • tried to remove monaco-editor from the babel-loader exclude to remedy this, but then the build would crash due to out-of-memory (OOM) heap allocation errors
          • as a result, only upgraded to 0.30 for now, can upgrade to latest once the build system itself is upgraded

Verification

Build passes, install succeeds

- 0.30 requires `monaco-editor-webpack-plugin` 6, per the [version matrix](https://github.com/microsoft/monaco-editor/tree/v0.41.0/webpack-plugin#version-matrix)
  - [0.29.0](https://github.com/microsoft/monaco-editor/blob/v0.41.0/CHANGELOG.md#0290-08102021) had a breaking change to `renderIndentGuides`; converted to the new option
    - `true` might not be necessary as it's the default, but `guides` is empty by default, so not 100% sure -- left it in for now to ensure the behavior remains the same after the upgrade

- 0.30 is the latest version I could upgrade to without requiring significant build changes
  - 0.31+ uses newer JS syntax per the [breaking changes](https://github.com/microsoft/monaco-editor/blob/v0.41.0/CHANGELOG.md#0310-10122021)
    - Per https://stackoverflow.com/a/70622999/3431180, this requires an updated parser, otherwise the Webpack build was getting similar errors as in the SO question
      - tried to remove `monaco-editor` from the `babel-loader` `exclude` to remedy this, but then the build would crash due to out-of-memory (OOM) heap allocation errors
        - as a result, only upgraded to 0.30 for now, can upgrade to latest once the build system is upgraded

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added type/dependencies PRs and issues specific to updating dependencies javascript Pull requests that update Javascript dependencies labels Aug 16, 2023
@agilgur5 agilgur5 changed the title deps(ui): upgrade monaco-editor to 0.30 chore(deps): upgrade monaco-editor to 0.30 Aug 16, 2023
@terrytangyuan terrytangyuan merged commit efb1181 into argoproj:master Aug 23, 2023
24 of 26 checks passed
@terrytangyuan
Copy link
Member

Did you verify that the editor still works?

@agilgur5
Copy link
Member Author

agilgur5 commented Aug 23, 2023

Oof that is a good question, I did not check that specifically. Most of my UI PRs modify the entire build, so build passing and just checking if the UI works is a decent verification, but this specific PR is actually for a very specific component of the UI.

My bad, let me double check that now. I hadn't gotten the UI dev server connected to the back-end build before, so didn't get past a spot check

@agilgur5
Copy link
Member Author

agilgur5 commented Aug 23, 2023

Took a bit, but got all the builds working together on my local machine.

Did need a small fix, see #11655

@agilgur5
Copy link
Member Author

Updated Webpack in #11628 which allowed this dep to be further updated.
Updated monaco-editor to latest 0.41.0 in #11710

terrytangyuan pushed a commit that referenced this pull request Oct 19, 2023
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui javascript Pull requests that update Javascript dependencies type/dependencies PRs and issues specific to updating dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants