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

Bump monaco-editor from 0.30.1 to 0.52.0 #9573

Open
Tracked by #9583
ananzh opened this issue Mar 19, 2025 · 2 comments
Open
Tracked by #9583

Bump monaco-editor from 0.30.1 to 0.52.0 #9573

ananzh opened this issue Mar 19, 2025 · 2 comments

Comments

@ananzh
Copy link
Member

ananzh commented Mar 19, 2025

Let me summarize how I bump the monaco-editor.

  • Upgrade monaco-editor to 0.52.0 in package.json and packages/osd-monaco
  • Go to packages/osd-monaco to run yarn install then yarn build --dev
  • Go back to OSD and run yarn install then yarn osd bootstrap

There are several issues:

Errors and Solutions

  1. Initial Issue:
    • The user had changed the monaco-editor version in package.json and packages/osd-monaco/package.json
    • When running yarn osd bootstrap, they encountered an error about regenerator-runtime
Couldn't find any versions for "regenerator-runtime" that matches "^0.52.0"
  1. First Error: Regenerator-runtime version mismatch

    • Error: Couldn't find any versions for "regenerator-runtime" that matches "^0.52.0"
    • Cause: The regenerator-runtime version in packages/osd-monaco/package.json was mistakenly set to "^0.52.0" (matching the monaco-editor version)
    • Fix: Changed regenerator-runtime version to "^0.13.3" to match the version in the main package.json
  2. Second Error: React-monaco-editor compatibility

    • Potential Issue: react-monaco-editor version might not be compatible with monaco-editor 0.52.0
    • Fix: Updated react-monaco-editor to version 0.52.0 in the main package.json
  3. Third Error: Modern JavaScript syntax in monaco-editor 0.52.0

    • Error: When building, encountered errors about newer JavaScript syntax features like optional chaining (?.), static initialization blocks (static { ... }), and nullish coalescing (??)
    • These features are not supported by the default webpack configuration
Module parse failed: Unexpected token (102:11)
You may need an appropriate loader to handle this file type
static { SuggestController_1 = this; }
  1. First Approach Considered: Remove node_modules exclusion

    • Proposed Solution: Remove the exclude: /node_modules/ line from webpack.config.js to allow babel-loader to process monaco-editor files
    • Pros: Simple change, would allow the existing babel configuration to process monaco-editor files
    • Cons: Would process all node_modules, potentially slowing down the build process and causing issues with other packages
    • TODO: I choose to keep the node_modules exclusion. Pls try to remove it and see if this can simply the bump.
  2. Second Approach Considered: Use different monaco-editor distribution formats

    • Options:
      • ESM (esm): ES modules (what was initially being used)
      • Development (dev): Development version (likely CommonJS)
      • Minified (min): Minified version (likely CommonJS)
    • We tried using the 'dev' version first, then the 'min' version
    • Pros: Might avoid the need to process monaco-editor files directly
    • Cons: Still encountered issues with imports not being found
    • TODO: Try to use esm
  3. Third Approach Considered: Create custom worker files

    • Proposed Solution: Create simplified worker files that don't rely on monaco-editor's internal modules
    • Pros: Avoids the need to process monaco-editor files directly
    • Cons: Might lose some functionality provided by monaco-editor's worker files
    • Implementation: We created simplified worker files for json.worker.ts and xjson.worker.ts
  4. Fourth Error: Numeric separators in monaco-editor 0.52.0

    • Error: Encountered errors about numeric separators (e.g., 100_000) in monaco-editor 0.52.0
    • Fix: Added the @babel/plugin-transform-numeric-separator plugin to handle this feature
Module parse failed: Identifier directly after number (78:17)
default: 20_000,
  1. Final Solution:
    • Keep the node_modules exclusion in webpack.config.js
    • Add a specific rule for monaco-editor files to process them with babel-loader
    • Add the necessary babel plugins to handle the newer JavaScript syntax features
    • Use simplified worker files to avoid issues with monaco-editor's internal modules
    • We tried several approaches to handle these modern syntax features
    • Eventually, succeeded by adding specific babel plugins and configuring webpack to process monaco-editor files

ESM vs CommonJS Comparison

  • ESM (ES Modules) is the more modern module system, standardized in ES6/ES2015
  • CommonJS is the older module system used by Node.js
  • Pros of ESM:
    • Static analysis at build time
    • Tree-shaking (dead code elimination)
    • Top-level await
    • More modern and future-proof
  • Pros of CommonJS:
    • Better compatibility with older tools and libraries
    • Dynamic imports at runtime
    • Simpler to use in some cases

Removing node_modules exclusion vs targeted processing:

  • Removing exclusion:
    • Pros: Simple change, would process all monaco-editor files
    • Cons: Would process all node_modules, potentially slowing down the build process
  • Targeted processing:
    • Pros: Only processes monaco-editor files, more efficient
    • Cons: Requires more configuration, might miss some files

Using different distribution formats vs processing ESM:

  • Using different formats (dev/min):
    • Pros: Might avoid the need for special processing
    • Cons: Might lose some functionality, might still require processing
  • Processing ESM:
    • Pros: Uses the most modern format, maintains all functionality
    • Cons: Requires more configuration to handle newer syntax features

Creating custom worker files vs using monaco-editor's worker files:

  • Custom worker files:
    • Pros: Simpler, avoids issues with monaco-editor's internal modules
    • Cons: Might lose some functionality provided by monaco-editor's worker files
  • Using monaco-editor's worker files:
    • Pros: Maintains all functionality
    • Cons: Requires more configuration to handle newer syntax features

Best Path Recommendation

For modern applications like OpenSearch-Dashboards, I recommend:

  1. Use ESM version of monaco-editor - It's the more modern and future-proof approach
  2. Process monaco-editor files with babel-loader - This handles newer syntax features while keeping the node_modules exclusion for other packages
  3. Add specific babel plugins as needed - This targets only the syntax features that need transformation
  4. Simplify worker files if necessary - This avoids complex dependencies while maintaining core functionality

This approach balances modern JavaScript practices with the constraints of the existing build system. It allows you to use the latest monaco-editor version while maintaining build performance and compatibility with the rest of the codebase.

@ananzh
Copy link
Member Author

ananzh commented Mar 19, 2025

Current progress is in this feature branch https://github.com/opensearch-project/OpenSearch-Dashboards/tree/bump-monaco-0.52.0

@ruchidh can you check the two TODOs and propose the solution to bump to 0.52.0? Let's use this feature branch and use this issue to record any errors and concerns.

@ananzh
Copy link
Member Author

ananzh commented Mar 19, 2025

The final solution maintained the use of the ESM version of monaco-editor while addressing the syntax challenges through targeted transpilation:

  • Kept the general exclude: /node_modules/ rule for most dependencies (for build performance)
  • Added a specific webpack rule that only targets monaco-editor files for babel processing
  • Applied the necessary babel plugins to handle modern syntax features
  • Simplified our monaco imports and worker files to avoid complex dependencies (not sure if this will causes any issues when start the editor)

So we can ignore the TODO for trying esm. But for removing node_modules, I think the impact is mxied

  • a more straightforward configuration (no special rules for specific packages)
  • accommodate future packages that use modern syntax, like other webpack configs in the repo
module: {
  rules: [
    {
      test: /\.(js|ts)$/,
      use: 'babel-loader',
      // No exclude: /node_modules/
    }
  ]
}
  • It aligns with the trend of treating node_modules as part of the build process rather than pre-built code

But if remove it, first of all it should increase the bundle size (not sure how big it is) and second we have exclude it since 0.27.0 not sure what would be the impact if we remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant