Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions components/webui/client/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions components/webui/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"@ant-design/v5-patch-for-react-19": "^1.0.3",
"@emotion/react": "^11.14.0",
"@emotion/styled": "^11.14.0",
"@monaco-editor/react": "^4.7.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the bundle size after adding Monaco?

One reason we avoided using this @monaco-editor/react was that the bundle size increases significantly as all Monaco assets were packed into the bundle. Is it still the case today?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i ran npm run build and this is the output:

> webui-client@0.1.0 build
> tsc -b && vite build

vite v6.2.6 building for production...
✓ 5687 modules transformed.
dist/index.html                     0.77 kB │ gzip:   0.41 kB
dist/assets/index-UZaMzv9P.css      2.73 kB │ gzip:   1.06 kB
dist/assets/index-CaK4Bu86.js   2,613.68 kB │ gzip: 838.16 kB

(!) Some chunks are larger than 500 kB after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 6.88s

previously, it was

vite v6.2.6 building for production...
✓ 5670 modules transformed.
dist/index.html                     0.77 kB │ gzip:   0.41 kB
dist/assets/index-CG-hk4vl.css      2.69 kB │ gzip:   1.04 kB
dist/assets/index-BdLJKTKx.js   2,593.66 kB │ gzip: 830.59 kB

so we can see the package size increase is not significant possibly because only the @monaco-editor/react are included. however, i wonder where the monaco-editor assets are added

Copy link
Member

@junhaoliao junhaoliao Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default, the loader-config loads Monaco assets from a CDN (on my end it uses https://cdn.jsdelivr.net/npm/monaco-editor): https://github.com/suren-atoyan/monaco-react/blob/master/README.md#loader-config

This would mean that if a user accesses the webui without access to the Internet or behind a country / corporate managed firewall, they would not be able to use the search page.

can we follow https://github.com/suren-atoyan/monaco-react/blob/master/README.md#use-monaco-editor-as-an-npm-package to config the monaco assets imports? (there's a Vite specific section)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on my end, if i just use their Vite example loader code, the bundle size bumped from 2.5MB to 15MB.

i think the issue is with

import * as monaco from 'monaco-editor';

if we use https://github.com/y-scope/yscope-log-viewer/blob/main/src/components/Editor/MonacoInstance/bootstrap.ts as a reference and import only what we need, the bundle size increase should be less than 972.2KB

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually never mind. we do need the whole bundle for the autocomplete to work. If you only want the editor, then we can't have autocomplete i think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import only what we need, the bundle size increase should be less than 972.2KB

this wont work since it will break the autocomplete, unless you find another thing to import that can fix. I was able to fix syntax highlighting but not autocomplete

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding this may fix, but not sure why import "monaco-editor/esm/vs/editor/editor.all.js"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now understand this 972.2KB is counting sizes of assets only in the "min" bundle and does not include any ESM APIs. Since we use the ESM APIs, we do need to import differently (more).

Let's see the comment at "monaco-config.ts" to reduce the scope of import. In the future, we should support lazy loading those.

"@mui/joy": "^5.0.0-beta.51",
"@sinclair/typebox": "^0.34.25",
"@tanstack/react-query": "^5.81.5",
Expand All @@ -26,6 +27,7 @@
"chartjs-adapter-dayjs-4": "^1.0.4",
"chartjs-plugin-zoom": "^2.2.0",
"dayjs": "^1.11.13",
"monaco-editor": "^0.52.2",
"react": "^19.0.0",
"react-chartjs-2": "^5.3.0",
"react-dom": "^19.0.0",
Expand Down
1 change: 1 addition & 0 deletions components/webui/client/public/settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"ClpStorageEngine": "clp",
"ClpQueryEngine": "native",
"MongoDbSearchResultsMetadataCollectionName": "results-metadata",
"SqlDbClpArchivesTableName": "clp_archives",
"SqlDbClpDatasetsTableName": "clp_datasets",
Expand Down
79 changes: 79 additions & 0 deletions components/webui/client/src/components/SqlEditor/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import {useEffect} from "react";

import {
Editor,
EditorProps,
useMonaco,
} from "@monaco-editor/react";
import {language as sqlLanguage} from "monaco-editor/esm/vs/basic-languages/sql/sql.js";

import "./monaco-config";


type SqlEditorProps = Omit<EditorProps, "language">;

/**
* Monaco editor with highlighting and autocomplete for SQL syntax.
*
* @param props
* @return
*/
const SqlEditor = (props: SqlEditorProps) => {
Copy link
Contributor

@hoophalab hoophalab Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't your issue, but when we resize the browser window's width below a threshold. The Monaco editor doesn't seem to update its width correctly, which can cause the "Run" button to get clipped.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a fix for this with css. so now fixed

const monaco = useMonaco();


useEffect(() => {
if (null === monaco) {
return () => {
};
}

// Adds autocomplete suggestions for SQL keywords on editor load
const provider = monaco.languages.registerCompletionItemProvider("sql", {
provideCompletionItems: (model, position) => {
const word = model.getWordUntilPosition(position);
const range = {
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
startColumn: word.startColumn,
endColumn: word.endColumn,
};
const suggestions = sqlLanguage.keywords.map((keyword: string) => ({
detail: "SQL Keyword",
insertText: `${keyword} `,
kind: monaco.languages.CompletionItemKind.Keyword,
label: keyword,
range: range,
}));

return {suggestions: suggestions};
},
});

return () => {
provider.dispose();
};
}, [monaco]);

return (
<Editor
language={"sql"}

// Use white background while loading (default is grey) so transition to editor with
// white background is less jarring.
loading={<div style={{backgroundColor: "white", height: "100%", width: "100%"}}/>}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just an idea - not strictly needed in this PR)
Would https://ant.design/components/skeleton#skeleton-demo-active be a better alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i tried it already! it looks worse since the grey lines in the skeleton appear and disapear. the plain white box just looks better

options={{
fontSize: 14,
lineNumbers: "on",
lineNumbersMinChars: 2,
minimap: {enabled: false},
overviewRulerBorder: false,
placeholder: "Enter your SQL query",
scrollBeyondLastLine: false,
wordWrap: "on",
}}
{...props}/>
);
};

export default SqlEditor;
21 changes: 21 additions & 0 deletions components/webui/client/src/components/SqlEditor/monaco-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* eslint-disable import/default */
import {loader} from "@monaco-editor/react";
import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
import EditorWorker from "monaco-editor/esm/vs/editor/editor.worker?worker";

import "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js";
import "monaco-editor/esm/vs/editor/editor.all.js";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can import only what we need, which saves another half MB:

Suggested change
/* eslint-disable import/default */
import {loader} from "@monaco-editor/react";
import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
import EditorWorker from "monaco-editor/esm/vs/editor/editor.worker?worker";
import "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js";
import "monaco-editor/esm/vs/editor/editor.all.js";
/* eslint-disable import/default, @stylistic/max-len */
import {loader} from "@monaco-editor/react";
import * as monaco from "monaco-editor/esm/vs/editor/editor.api";
import EditorWorker from "monaco-editor/esm/vs/editor/editor.worker?worker";
import "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js";
import "monaco-editor/esm/vs/editor/contrib/clipboard/browser/clipboard.js";
import "monaco-editor/esm/vs/editor/contrib/contextmenu/browser/contextmenu.js";
import "monaco-editor/esm/vs/editor/contrib/find/browser/findController.js";
import "monaco-editor/esm/vs/editor/contrib/wordHighlighter/browser/wordHighlighter.js";
import "monaco-editor/esm/vs/editor/contrib/suggest/browser/suggestController.js";
import "monaco-editor/esm/vs/editor/contrib/placeholderText/browser/placeholderText.contribution.js";
/* eslint-enable @stylistic/max-len */

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've found ways to lazy load the monaco assets but the code is not very organized at this point. i'll investigate further in the future

@coderabbitai create an issue to support lazy loading of the monaco assets

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

I'll create an issue to track the lazy loading optimization for Monaco assets.

@junhaoliao I've created an issue to track the lazy loading optimization for Monaco Editor assets. This will help organize the future work to improve page load performance while maintaining the current SQL functionality.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for lazy loading Monaco assets
gh issue create \
  --title "Optimize Monaco Editor with lazy loading of assets" \
  --body "## Description

Currently, the Monaco Editor implementation loads all assets upfront, which could be optimized with lazy loading to improve initial page load performance.

## Context

The Monaco Editor was recently added in PR #1108 to support SQL syntax highlighting and autocomplete. While investigating optimization opportunities, lazy loading techniques were identified but the implementation needs to be better organized.

## Goals

- Implement lazy loading for Monaco Editor assets
- Improve initial page load performance 
- Maintain current SQL autocomplete and syntax highlighting functionality
- Ensure code organization and maintainability

## References

- Original PR: https://github.com/y-scope/clp/pull/1108
- Discussion: https://github.com/y-scope/clp/pull/1108#discussion_r2220658679

## Related Files

- \`components/webui/client/src/components/SqlEditor/monaco-config.ts\`
- \`components/webui/client/vite.config.ts\`" \
  --assignee junhaoliao

Length of output: 1084


@junhaoliao Perfect! I've successfully created issue #1120 to track the lazy loading optimization for Monaco Editor assets. The issue includes the context from this PR, your investigation findings, and references to the relevant files and discussion.

You can view the issue here: #1120

The issue is assigned to you and captures the goals of implementing lazy loading while maintaining the current SQL functionality and ensuring good code organization.


🧠 Learnings used
Learnt from: davemarco
PR: y-scope/clp#1108
File: components/webui/client/src/components/SqlEditor/monaco-config.ts:6-8
Timestamp: 2025-07-21T14:37:57.611Z
Learning: In Monaco Editor configurations for SQL editing, the import of "monaco-editor/esm/vs/editor/editor.all.js" is required for SQL autocomplete functionality to work properly, even when using "editor.api" and "sql.contribution.js". Removing "editor.all.js" breaks the autocomplete feature.


Copy link
Contributor

@coderabbitai coderabbitai bot Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Redundant heavy imports inflate bundle size.
editor.api already registers the editor core; pulling in editor.all.js adds the full feature set again, doubling payload. Unless you explicitly need the legacy “all” bundle, drop it and rely on targeted feature contributions (sql.contribution.js).

🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlEditor/monaco-config.ts around
lines 6 to 8, the import of "monaco-editor/esm/vs/editor/editor.all.js" is
redundant and inflates the bundle size because "editor.api" already registers
the editor core. Remove the import of "editor.all.js" and rely solely on
targeted feature contributions like "sql.contribution.js" to reduce payload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the autocomplete dosent work without it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davemarco Thank you for the clarification! You're absolutely right - if the autocomplete functionality breaks without the editor.all.js import, then it's definitely necessary to keep it. The bundle size trade-off is worth it for the functionality, especially since SQL autocomplete is a key feature mentioned in your PR objectives.

I appreciate you testing this and providing the practical feedback. The Monaco Editor's dependency requirements can be tricky, and your real-world experience is invaluable.


✏️ Learnings added
Learnt from: davemarco
PR: y-scope/clp#1108
File: components/webui/client/src/components/SqlEditor/monaco-config.ts:6-8
Timestamp: 2025-07-21T14:37:57.611Z
Learning: In Monaco Editor configurations for SQL editing, the import of "monaco-editor/esm/vs/editor/editor.all.js" is required for SQL autocomplete functionality to work properly, even when using "editor.api" and "sql.contribution.js". Removing "editor.all.js" breaks the autocomplete feature.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


self.MonacoEnvironment = {
/**
* Creates a web worker for Monaco Editor.
*
* @return
*/
getWorker () {
return new EditorWorker();
},
};

loader.config({monaco});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
declare module "monaco-editor/esm/vs/basic-languages/sql/sql.js" {
import {languages} from "monaco-editor/esm/vs/editor/editor.api";


interface SqlLanguageDefinition extends languages.IMonarchLanguage {
keywords: string[];
}

export const language: SqlLanguageDefinition;
}
11 changes: 11 additions & 0 deletions components/webui/client/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ enum CLP_STORAGE_ENGINES {
CLP_S = "clp-s",
}

/**
* Query engine options.
*/
enum CLP_QUERY_ENGINES {
NATIVE = "native",
PRESTO = "presto",
}

const SETTINGS_STORAGE_ENGINE = settings.ClpStorageEngine as CLP_STORAGE_ENGINES;
const SETTINGS_QUERY_ENGINE = settings.ClpQueryEngine as CLP_QUERY_ENGINES;

/**
* Stream type based on the storage engine.
Expand All @@ -19,7 +28,9 @@ const STREAM_TYPE = CLP_STORAGE_ENGINES.CLP === SETTINGS_STORAGE_ENGINE ?
"json";

export {
CLP_QUERY_ENGINES,
CLP_STORAGE_ENGINES,
SETTINGS_QUERY_ENGINE,
SETTINGS_STORAGE_ENGINE,
STREAM_TYPE,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import {CaretRightOutlined} from "@ant-design/icons";
import {
Button,
Tooltip,
} from "antd";

import useSearchStore from "../../../SearchState/index";


/**
* Renders a button to run the SQL query.
*
* @return
*/
const RunButton = () => {
const queryString = useSearchStore((state) => state.queryString);

const isQueryStringEmpty = "" === queryString;
const tooltipTitle = isQueryStringEmpty ?
"Enter SQL query to run" :
"";

return (
<Tooltip title={tooltipTitle}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a consistent look, we should disable this button when the querystring is empty, just like we did with the submit button.

<Button
color={"green"}
disabled={isQueryStringEmpty}
icon={<CaretRightOutlined/>}
size={"large"}
variant={"solid"}
>
Run
</Button>
</Tooltip>
);
};

export default RunButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/* Allows the editor to shrink when page width decreases */
.input {
width: 100%;
min-width: 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {useCallback} from "react";

import SqlEditor from "../../../../../components/SqlEditor";
import useSearchStore from "../../../SearchState/index";
import styles from "./index.module.css";


/**
* Renders SQL query input.
*
* @return
*/
const SqlQueryInput = () => {
const handleChange = useCallback((value: string | undefined) => {
const {updateQueryString} = useSearchStore.getState();
updateQueryString(value || "");
}, []);

return (
<div className={styles["input"] || ""}>
<SqlEditor
height={"150px"}
onChange={handleChange}/>
</div>
);
};

export default SqlQueryInput;
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
SETTINGS_STORAGE_ENGINE,
} from "../../../../../config";
import {computeTimelineConfig} from "../../../SearchResults/SearchResultsTimeline/utils";
import useSearchStore, {SEARCH_STATE_DEFAULT} from "../../../SearchState/index";
import useSearchStore from "../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../SearchState/typings";
import {handleQuerySubmit} from "../../search-requests";
import styles from "./index.module.css";
Expand Down Expand Up @@ -63,7 +63,7 @@ const SubmitButton = () => {
selectDataset,
updateCachedDataset]);

const isQueryStringEmpty = queryString === SEARCH_STATE_DEFAULT.queryString;
const isQueryStringEmpty = "" === queryString;

// Submit button must be disabled if there are no datasets since clp-s requires dataset option
// for queries.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import {
CLP_QUERY_ENGINES,
CLP_STORAGE_ENGINES,
SETTINGS_QUERY_ENGINE,
SETTINGS_STORAGE_ENGINE,
} from "../../../config";
import Dataset from "./Dataset";
import styles from "./index.module.css";
import RunButton from "./Presto/RunButton";
import SqlQueryInput from "./Presto/SqlQueryInput";
import QueryInput from "./QueryInput";
import SearchButton from "./SearchButton";
import TimeRangeInput from "./TimeRangeInput";
Expand All @@ -27,10 +31,21 @@ const SearchControls = () => {
return (
<form onSubmit={handleSubmit}>
<div className={styles["searchControlsContainer"]}>
{CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE && <Dataset/>}
<QueryInput/>
<TimeRangeInput/>
<SearchButton/>
{SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.NATIVE ?
(
<>
{CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE && <Dataset/>}
<QueryInput/>
<TimeRangeInput/>
<SearchButton/>
</>
) :
(
<>
<SqlQueryInput/>
<RunButton/>
Copy link
Contributor

@hoophalab hoophalab Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the Monaco width issue, what do you think about moving the run button below the SQL editor and aligning it to the right?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it at first, but it will complicate searchQueryStatus. Like right now searchQueryStatus is a separate component under SearchControls. I think it looks weird to have searchQueryStatus under the button. I though about putting searchQueryStatus next to the button, but it will just be mess to render searchQueryStatus in different locations for "presto" and "native". I thought just putting the button on the right makes "native" and "presto" have similiar layout simplifying the codebase

</>
)}
</div>
</form>
);
Expand Down
1 change: 1 addition & 0 deletions components/webui/client/src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import axios from "axios";

type Settings = {
ClpStorageEngine: string;
ClpQueryEngine: string;
MongoDbSearchResultsMetadataCollectionName: string;
SqlDbClpArchivesTableName: string;
SqlDbClpDatasetsTableName: string;
Expand Down
7 changes: 7 additions & 0 deletions components/webui/client/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ export default defineConfig({
base: "./",
build: {
target: "esnext",
rollupOptions: {
output: {
manualChunks: {
"monaco-editor": ["monaco-editor"],
},
},
},
},
plugins: [
react(),
Expand Down
Loading