feat(webui): Integrate Monaco editor for future Presto SQL query input. #1108
Conversation
WalkthroughThis change introduces SQL query editing functionality to the web UI client by adding a Monaco-based SQL editor, related components, and configuration for supporting multiple query engines. It updates dependencies, settings, and type definitions, and conditionally renders input controls based on the selected query engine. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchControls
participant SqlQueryInput
participant SqlEditor
participant RunButton
User->>SearchControls: Open search page
SearchControls->>SearchControls: Check ClpQueryEngine setting
alt Engine is PRESTO
SearchControls->>SqlQueryInput: Render SQL input
SearchControls->>RunButton: Render Run button
User->>SqlEditor: Type SQL query
User->>RunButton: Click Run
else Engine is NATIVE
SearchControls->>QueryInput: Render Query input
SearchControls->>SearchButton: Render Search button
User->>QueryInput: Enter query
User->>SearchButton: Click Search
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (4)📓 Common learningscomponents/webui/client/src/components/SqlEditor/monaco-loader.ts (1)Learnt from: davemarco components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (8)Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: hoophalab Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: junhaoliao components/webui/client/src/components/SqlEditor/index.tsx (9)Learnt from: davemarco Learnt from: junhaoliao Learnt from: junhaoliao Learnt from: gibber9809 Learnt from: junhaoliao Learnt from: Bill-hbrhbr Learnt from: junhaoliao Learnt from: AVMatthews Learnt from: quinntaylormitchell 🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/webui/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
components/webui/client/package.json(2 hunks)components/webui/client/public/settings.json(1 hunks)components/webui/client/src/components/SqlEditor/index.tsx(1 hunks)components/webui/client/src/components/SqlEditor/monaco-sql.d.ts(1 hunks)components/webui/client/src/config/index.ts(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/index.tsx(2 hunks)components/webui/client/src/settings.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (9)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/client/public/settings.json (1)
Learnt from: LinZhihao-723
PR: y-scope/clp#873
File: components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp:320-324
Timestamp: 2025-05-05T01:12:18.561Z
Learning: In the CLP codebase, the `m_case_sensitive_search` flag is used only for actual string value comparisons during query evaluation, not for schema key name matching. Schema keys are always compared case-sensitively regardless of this flag's setting.
components/webui/client/src/components/SqlEditor/monaco-sql.d.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/components/SqlEditor/index.tsx (1)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/pages/SearchPage/SearchControls/index.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/config/index.ts (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: LinZhihao-723
PR: y-scope/clp#873
File: components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp:320-324
Timestamp: 2025-05-05T01:12:18.561Z
Learning: In the CLP codebase, the `m_case_sensitive_search` flag is used only for actual string value comparisons during query evaluation, not for schema key name matching. Schema keys are always compared case-sensitively regardless of this flag's setting.
components/webui/client/package.json (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
🧬 Code Graph Analysis (2)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx (1)
components/webui/client/src/pages/SearchPage/SearchState/index.tsx (1)
SEARCH_STATE_DEFAULT(153-153)
components/webui/client/src/pages/SearchPage/SearchControls/index.tsx (1)
components/webui/client/src/config/index.ts (4)
SETTINGS_QUERY_ENGINE(33-33)CLP_QUERY_ENGINES(31-31)CLP_STORAGE_ENGINES(32-32)SETTINGS_STORAGE_ENGINE(34-34)
🔇 Additional comments (14)
components/webui/client/package.json (1)
19-19: Confirm dependency versions and perform security auditBoth
@monaco-editor/react(4.7.0) andmonaco-editor(0.52.2) are up-to-date with the latest published releases. However, no lockfile exists in the repo, so a security audit couldn’t run. Please:
- Generate a lockfile (for example, run
npm install --package-lock-only).- Execute
npm audit --audit-level=moderate.- Address any vulnerabilities before merging.
components/webui/client/src/settings.ts (1)
6-6: LGTM!The
ClpQueryEngineproperty addition follows the existing pattern and correctly extends theSettingstype to support the new query engine configuration.components/webui/client/public/settings.json (1)
3-3: LGTM!The default value "native" maintains backward compatibility while enabling the new query engine selection feature. The JSON structure is correct and follows the existing pattern.
components/webui/client/src/config/index.ts (3)
12-18: LGTM!The
CLP_QUERY_ENGINESenum follows the established pattern and provides clear type safety for query engine selection.
21-21: LGTM!The
SETTINGS_QUERY_ENGINEconstant correctly applies type casting following the same pattern asSETTINGS_STORAGE_ENGINE.
31-31: LGTM!The exports are correctly updated to include the new enum and constant, maintaining consistency with the module's export pattern.
Also applies to: 33-33
components/webui/client/src/components/SqlEditor/monaco-sql.d.ts (1)
1-10: LGTM!The TypeScript declaration correctly extends the Monaco SQL language definition with the
keywordsproperty needed for autocomplete functionality. The module path and interface extension follow proper TypeScript patterns.components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (1)
12-23: LGTM! Clean component structure with proper callback handling.The component follows React best practices with proper use of
useCallbackto prevent unnecessary re-renders and handles the undefined case gracefully.components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx (1)
16-38: LGTM! Clean component with proper state management and conditional UI.The component correctly integrates with the search store and provides appropriate user feedback through the conditional tooltip.
components/webui/client/src/pages/SearchPage/SearchControls/index.tsx (2)
37-37: Good: Maintained the existing Dataset conditional logic.The nested conditional for the Dataset component within the native branch preserves the existing behavior while adding the new query engine support.
34-48: LGTM! Clean implementation of conditional query engine UI.The conditional rendering properly separates native and Presto query engine controls while maintaining the existing form structure and submission handling.
components/webui/client/src/components/SqlEditor/index.tsx (3)
23-54: Excellent cleanup pattern in useEffect.The useEffect properly handles the Monaco instance lifecycle with correct cleanup to prevent memory leaks. The completion provider disposal is properly handled.
56-74: LGTM! Comprehensive editor configuration with thoughtful defaults.The editor options are well-configured for SQL editing with appropriate settings for font size, line numbers, word wrap, and user experience improvements like the custom loading component.
11-11: Clean type definition that properly constrains the language prop.The
Omit<EditorProps, "language">type correctly prevents consumers from overriding the fixed SQL language setting.
| <SqlEditor | ||
| height={"150px"} | ||
| onChange={handleChange}/> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider making the height prop configurable.
The hardcoded height of "150px" might be better as a configurable prop to improve reusability.
-const SqlQueryInput = () => {
+interface SqlQueryInputProps {
+ height?: string;
+}
+
+const SqlQueryInput = ({height = "150px"}: SqlQueryInputProps) => {
const handleChange = useCallback((value: string | undefined) => {
const {updateQueryString} = useSearchStore.getState();
updateQueryString(value || "");
}, []);
return (
<SqlEditor
- height={"150px"}
+ height={height}
onChange={handleChange}/>
);
};🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
around lines 19 to 21, the SqlEditor component has a hardcoded height of
"150px". To improve reusability, add a height prop to the SqlQueryInput
component that defaults to "150px" and pass this prop to SqlEditor instead of
the hardcoded value. This allows the height to be configurable when using
SqlQueryInput.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
Outdated
Show resolved
Hide resolved
| return ( | ||
| <Tooltip title={tooltipTitle}> | ||
| <Button | ||
| className={styles["gradientButton"] || ""} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove defensive fallback for CSS className.
The defensive || "" suggests uncertainty about the style's existence. If the style is expected to exist, remove the fallback; if it's optional, make it explicit.
- className={styles["gradientButton"] || ""}
+ className={styles["gradientButton"]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={styles["gradientButton"] || ""} | |
| className={styles["gradientButton"]} |
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
at line 29, remove the defensive fallback `|| ""` from the className assignment
since the style is expected to exist. Change
`className={styles["gradientButton"] || ""}` to just
`className={styles["gradientButton"]}` to clearly express that the style should
always be present.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
Outdated
Show resolved
Hide resolved
| * @param props | ||
| * @return | ||
| */ | ||
| const SqlEditor = (props: SqlEditorProps) => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I found a fix for this with css. so now fixed
| const RunButton = () => { | ||
| const queryString = useSearchStore((state) => state.queryString); | ||
|
|
||
| const isQueryStringEmpty = queryString === SEARCH_STATE_DEFAULT.queryString; |
There was a problem hiding this comment.
isQueryStringEmpty is confusing.
| const isQueryStringEmpty = queryString === SEARCH_STATE_DEFAULT.queryString; | |
| const isQueryStringEmpty = queryString === ""; |
I find it counter-intuitive to check for equality against a default value, especially if we set default to something frequently used later (e.g., SEARCH_STATE_DEFAULT.queryString === "SELECT * FROM default").
There was a problem hiding this comment.
sure i changed this here and the other submit button
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
Outdated
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Show resolved
Hide resolved
| return ( | ||
| <Tooltip title={tooltipTitle}> | ||
| <Button | ||
| className={styles["gradientButton"] || ""} |
There was a problem hiding this comment.
Let's move the gradientButton's CSS file to a shared folder, maybe in src/components. We shouldn't have one component's styles depends on another. Otherwise, if someone updates the submit button later, they'll unintentionally change the run button's appearance too.
There was a problem hiding this comment.
Or, ideally, we create a reusable GradientButton component and let both submit button and run button use GradientButton.
There was a problem hiding this comment.
Instead of adding overhead of a custom component, i just made this button solid green.
| } | ||
|
|
||
| return ( | ||
| <Tooltip title={tooltipTitle}> |
There was a problem hiding this comment.
For a consistent look, we should disable this button when the querystring is empty, just like we did with the submit button.
| ( | ||
| <> | ||
| <SqlQueryInput/> | ||
| <RunButton/> |
There was a problem hiding this comment.
Besides the Monaco width issue, what do you think about moving the run button below the SQL editor and aligning it to the right?
There was a problem hiding this comment.
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
| * @param props | ||
| * @return | ||
| */ | ||
| const SqlEditor = (props: SqlEditorProps) => { |
There was a problem hiding this comment.
I found a fix for this with css. so now fixed
| const RunButton = () => { | ||
| const queryString = useSearchStore((state) => state.queryString); | ||
|
|
||
| const isQueryStringEmpty = queryString === SEARCH_STATE_DEFAULT.queryString; |
There was a problem hiding this comment.
sure i changed this here and the other submit button
| return ( | ||
| <Tooltip title={tooltipTitle}> | ||
| <Button | ||
| className={styles["gradientButton"] || ""} |
There was a problem hiding this comment.
Instead of adding overhead of a custom component, i just made this button solid green.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Show resolved
Hide resolved
| ( | ||
| <> | ||
| <SqlQueryInput/> | ||
| <RunButton/> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (1)
20-20: Fix ESLint formatting violations.The pipeline shows formatting issues that need to be addressed.
- <div className={styles[`editorWrapper`] || "" }> + <div className={styles["editorWrapper"] || ""}>components/webui/client/src/components/SqlEditor/index.tsx (2)
24-27: Simplify early return in useEffect.The explicit empty function return is unnecessary.
useEffect(() => { if (null === monaco) { - return () => { - }; + return; }
39-39: Add defensive programming for SQL keywords access.Consider adding a safety check for potential runtime errors.
- const suggestions = sqlLanguage.keywords.map((keyword: string) => ({ + const suggestions = (sqlLanguage.keywords || []).map((keyword: string) => ({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/webui/client/src/components/SqlEditor/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.module.css(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (4)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/client/src/components/SqlEditor/index.tsx (3)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: gibber9809
PR: y-scope/clp#672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx (5)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#868
File: components/clp-package-utils/clp_package_utils/scripts/start_clp.py:869-870
Timestamp: 2025-06-10T05:56:56.572Z
Learning: The team prefers keeping simple conditional logic inline rather than extracting it into helper functions, especially for straightforward operations like string concatenation based on storage engine type.
Learnt from: junhaoliao
PR: y-scope/clp#988
File: components/log-viewer-webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx:34-37
Timestamp: 2025-06-09T17:48:56.024Z
Learning: In the y-scope/clp project, prefer `false == <expression>` rather than `!<expression>` for boolean expressions in TypeScript/JavaScript files, as specified in the coding guidelines.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#1078
File: components/webui/server/src/plugins/MongoSocketIoServer/index.ts:105-108
Timestamp: 2025-07-08T17:20:02.560Z
Learning: The y-scope/clp codebase has a 100-character line length limit that takes precedence over other style preferences like template literals when they conflict.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
🪛 GitHub Actions: clp-lint
components/webui/client/src/components/SqlEditor/index.tsx
[error] 63-63: ESLint: Multiline props must be listed after all other props (@stylistic/jsx-sort-props)
[warning] 72-72: ESLint: Expected object keys to be in natural ascending order. 'automaticLayout' should be before 'wordWrap' (sort-keys)
[error] 74-74: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
[error] 18-18: ESLint: Expected literal to be on the left side of === (yoda)
[error] 19-19: ESLint: Expected newline between test and consequent of ternary expression (@stylistic/multiline-ternary)
[error] 19-19: ESLint: Expected newline between consequent and alternate of ternary expression (@stylistic/multiline-ternary)
[error] 25-25: ESLint: Props should be sorted alphabetically (@stylistic/jsx-sort-props)
[error] 25-25: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
[error] 26-26: ESLint: There should be no space before '=' (@stylistic/jsx-equals-spacing)
[error] 26-26: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
[error] 27-27: ESLint: Props should be sorted alphabetically (@stylistic/jsx-sort-props)
[error] 28-28: ESLint: Props should be sorted alphabetically (@stylistic/jsx-sort-props)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
[warning] 20-20: ESLint: Strings must use doublequote (@stylistic/quotes)
[error] 20-20: ESLint: There should be no space before '}' (@stylistic/jsx-curly-spacing)
🔇 Additional comments (4)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.module.css (1)
1-4: LGTM! Clean CSS module for editor wrapper.The styling is appropriate for an editor wrapper component. The
min-width: 0prevents flex overflow issues, ensuring the editor resizes properly within its container.components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (1)
14-17: Store access pattern is appropriate per team preference.The use of
getState()for non-reactive callbacks is intentional and aligns with the team's established pattern for this type of interaction.components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx (1)
18-19: Fix ESLint formatting violations.Multiple ESLint errors need to be addressed per the coding guidelines.
- const isQueryStringEmpty = queryString === ""; - const tooltipTitle = isQueryStringEmpty ? "Enter SQL query to run" : ""; + const isQueryStringEmpty = "" === queryString; + const tooltipTitle = isQueryStringEmpty + ? "Enter SQL query to run" + : "";⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:16-23 Timestamp: 2024-11-21T15:51:33.203Z Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:35-41 Timestamp: 2024-11-19T19:52:43.429Z Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.components/webui/client/src/components/SqlEditor/index.tsx (1)
57-75: Fix ESLint formatting violations.Multiple prop and key ordering issues need to be addressed.
<Editor language={"sql"} loading={<div style={{backgroundColor: "white", height: "100%", width: "100%"}}/>} + width={"100%"} options={{ + automaticLayout: true, fontSize: 14, lineNumbers: "on", lineNumbersMinChars: 2, minimap: {enabled: false}, overviewRulerBorder: false, placeholder: "Enter your SQL query", scrollBeyondLastLine: false, wordWrap: "on", - automaticLayout: true, }} - width="100%" {...props}/>⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#937 File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24 Timestamp: 2025-05-29T20:33:40.653Z Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.Learnt from: junhaoliao PR: y-scope/clp#1078 File: components/webui/server/src/plugins/MongoSocketIoServer/index.ts:105-108 Timestamp: 2025-07-08T17:20:02.560Z Learning: The y-scope/clp codebase has a 100-character line length limit that takes precedence over other style preferences like template literals when they conflict.Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:16-23 Timestamp: 2024-11-21T15:51:33.203Z Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (2)
14-17: LGTM! Callback implementation aligns with team decision.The use of
getState()directly in the callback was already discussed in previous reviews, and the team decided this approach is preferred for callbacks to avoid reactive dependencies.
21-23: Hardcoded height remains inflexible.The hardcoded height of "150px" was already flagged in previous reviews as a reusability concern. Consider the previous suggestion to make it configurable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.module.css(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (4)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-check (macos-latest)
🔇 Additional comments (3)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.module.css (1)
1-5: LGTM! Clean and well-documented responsive CSS.The CSS implementation is correct and the comment clearly explains the purpose of the
min-width: 0property for responsive behavior.components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx (2)
1-5: LGTM! Clean imports and structure.The imports are well-organized and follow the established patterns in the codebase.
8-12: LGTM! Good JSX documentation.The component documentation is clear and follows the established format. Good that no return type annotation is used, which aligns with the codebase preferences.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
lgtm.
We might want to rethink the color matching, but let's save all those cosmetic touch-ups for a dedicated PR once all the functionalities are wrapped up.
Validations:
- set the query engine in settings.json to presto.
- there are no obvious glitches in the new UI.
| @@ -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", | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
is seems reasonable here? https://bundlephobia.com/package/@monaco-editor/react@4.7.0
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
adding this may fix, but not sure why import "monaco-editor/esm/vs/editor/editor.all.js"
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
i tested the UI and it looks very cool
i'll approve once #1108 (comment) is addressed
|
|
||
| // 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%"}}/>} |
There was a problem hiding this comment.
(just an idea - not strictly needed in this PR)
Would https://ant.design/components/skeleton#skeleton-demo-active be a better alternative?
There was a problem hiding this comment.
i tried it already! it looks worse since the grey lines in the skeleton appear and disapear. the plain white box just looks better
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (5)
components/webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts (1)
15-16: Use a selector when reading from the Zustand store.
const {searchJobId} = useSearchStore();subscribes the component to the entire store, causing unnecessary re-renders.
Switch to a selector to keep renders minimal:-const {searchJobId} = useSearchStore(); +const searchJobId = useSearchStore((state) => state.searchJobId);components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts (1)
21-29: Prevent stale function reference in effect deps.
updateSearchUiStateis a stable method on the store; including it in theuseEffectdependency list forces the hook to re-subscribe whenever the store changes.
Capture it once outside the component or reference it viauseSearchStore.getState():-const {updateSearchUiState} = useSearchStore(); +const updateSearchUiState = useSearchStore.getState().updateSearchUiState;and remove it from the dependency array.
components/webui/client/src/api/socket/MongoSocketCollection.ts (1)
21-23: Spelling: “Initialises”.Minor typo in the JSDoc –
Initalizes→Initialises. Fix for readability.- * Initalizes socket connection to a MongoDB collection on the server. + * Initialises socket connection to a MongoDB collection on the server.components/webui/client/src/api/socket/SocketSingleton.ts (1)
22-24: Follow codebase boolean-negation guideline.Coding guidelines prefer
false == <expr>over!<expr>.
Update the conditional accordingly:- if (!sharedSocket) { + if (false == sharedSocket) {taskfile.yaml (1)
397-407: Checksum validation forcommon/node_moduleslacks the same exclusion list used for other dirs
client/node_modulesexcludes transient folders (.tmp,.vite-temp) when computing/validating checksums, but those exclusions are not applied tocommon/node_modules. For consistency and to avoid churn, reuse*webui_node_modules_checksum_exclude_patterns:- - task: "utils:checksum:validate" + - task: "utils:checksum:validate" vars: CHECKSUM_FILE: "{{.G_WEBUI_COMMON_NODE_MODULES_CHECKSUM_FILE}}" - INCLUDE_PATTERNS: ["{{.COMMON_OUTPUT_DIR}}"] + INCLUDE_PATTERNS: ["{{.COMMON_OUTPUT_DIR}}"] + EXCLUDE_PATTERNS: *webui_node_modules_checksum_exclude_patterns
♻️ Duplicate comments (3)
components/webui/client/package.json (2)
19-19: Verify Monaco editor configuration to avoid CDN dependencies.Based on previous discussions, the Monaco editor may load assets from CDN by default, which could cause issues for users without internet access or behind firewalls.
Please ensure the Monaco editor is configured to bundle assets locally as discussed in previous reviews, following the Vite-specific configuration to avoid CDN dependencies.
31-31: Consider selective Monaco editor imports to reduce bundle size.Previous analysis showed the bundle size could increase significantly with full Monaco imports. Consider using selective imports as referenced in the yscope-log-viewer implementation to minimize the bundle size impact.
Reference the selective import approach from
https://github.com/y-scope/yscope-log-viewer/blob/main/src/components/Editor/MonacoInstance/bootstrap.tsto reduce bundle size impact.components/webui/client/src/components/SqlEditor/index.tsx (1)
24-28: Redundant empty-function return is still presentThe early return inside
useEffectunnecessarily returns an empty function; returningundefined(or omitting the return) achieves the same cleanup semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/webui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py(2 hunks)components/webui/client/package.json(1 hunks)components/webui/client/public/settings.json(1 hunks)components/webui/client/src/api/socket/MongoSocketCollection.ts(1 hunks)components/webui/client/src/api/socket/MongoSocketCursor.ts(1 hunks)components/webui/client/src/api/socket/SocketSingleton.ts(1 hunks)components/webui/client/src/components/SqlEditor/index.tsx(1 hunks)components/webui/client/src/components/SqlEditor/monaco-config.ts(1 hunks)components/webui/client/src/components/SqlEditor/monaco-sql.d.ts(1 hunks)components/webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts(1 hunks)components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts(1 hunks)components/webui/client/tsconfig/tsconfig.app.json(0 hunks)components/webui/client/vite.config.ts(0 hunks)components/webui/common/package.json(1 hunks)components/webui/common/tsconfig.json(1 hunks)components/webui/package.json(1 hunks)components/webui/server/package.json(2 hunks)components/webui/server/src/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts(1 hunks)components/webui/server/src/plugins/app/socket/MongoSocketIoServer/index.ts(1 hunks)components/webui/server/src/plugins/app/socket/MongoSocketIoServer/typings.ts(1 hunks)components/webui/server/src/routes/api/search/index.ts(1 hunks)components/webui/server/src/routes/api/search/typings.ts(1 hunks)components/webui/server/src/routes/api/search/utils.ts(1 hunks)components/webui/server/src/routes/static.ts(1 hunks)components/webui/server/tsconfig.json(0 hunks)taskfile.yaml(11 hunks)
💤 Files with no reviewable changes (3)
- components/webui/server/tsconfig.json
- components/webui/client/tsconfig/tsconfig.app.json
- components/webui/client/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (23)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts (4)
Learnt from: hoophalab
PR: y-scope/clp#1108
File: components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx:15-15
Timestamp: 2025-07-18T20:00:50.261Z
Learning: In the y-scope/clp React webui client codebase, for Zustand store usage: use `useStore.getState().method` for callbacks since the output is not reactive and doesn't need state as a dependency in the hook, and use `useStore((state) => state.property)` with proper selectors for reactive components that need to re-render when state changes.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: junhaoliao
PR: y-scope/clp#962
File: components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx:22-26
Timestamp: 2025-06-09T17:15:30.309Z
Learning: When using Zustand stores in React components, always use specific selectors (e.g., `useStore(state => state.property)`) instead of destructuring the entire state object to prevent unnecessary re-renders. This is especially important as state stores grow in complexity.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/server/src/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts (4)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:79-83
Timestamp: 2025-05-05T18:03:50.472Z
Learning: In MongoWatcherCollection, the unsubscribe method is intentionally designed to remove only one instance of a connection ID from the subscribers array using removeItemFromArray. This allows for multiple subscriptions from the same connection, with each unsubscribe call removing only one subscription at a time.
components/webui/server/src/routes/api/search/utils.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/server/src/plugins/app/socket/MongoSocketIoServer/index.ts (5)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:79-83
Timestamp: 2025-05-05T18:03:50.472Z
Learning: In MongoWatcherCollection, the unsubscribe method is intentionally designed to remove only one instance of a connection ID from the subscribers array using removeItemFromArray. This allows for multiple subscriptions from the same connection, with each unsubscribe call removing only one subscription at a time.
components/webui/server/src/routes/api/search/typings.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/public/settings.json (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: LinZhihao-723
PR: y-scope/clp#873
File: components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp:320-324
Timestamp: 2025-05-05T01:12:18.561Z
Learning: In the CLP codebase, the `m_case_sensitive_search` flag is used only for actual string value comparisons during query evaluation, not for schema key name matching. Schema keys are always compared case-sensitively regardless of this flag's setting.
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.
components/webui/server/src/routes/api/search/index.ts (2)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/package.json (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
components/webui/client/src/api/socket/SocketSingleton.ts (3)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/api/socket/MongoSocketCollection.ts (5)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#880
File: components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/MongoWatcherCollection.ts:196-200
Timestamp: 2025-05-05T18:08:23.313Z
Learning: In the MongoSocketIoServer plugin, change streams are designed to be closed only through explicit client unsubscribe actions rather than having automatic cleanup on error or unexpected close events. This design choice puts the responsibility on the client to properly unsubscribe.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:25-43
Timestamp: 2025-05-09T18:06:33.042Z
Learning: Socket.IO connections in JavaScript require explicit disconnection by calling socket.disconnect() and are not automatically cleaned up when the object is garbage collected.
components/webui/client/src/api/socket/MongoSocketCursor.ts (3)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
components/webui/server/src/routes/static.ts (1)
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (6)
Learnt from: haiqi96
PR: y-scope/clp#594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
components/webui/common/package.json (3)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#647
File: components/log-viewer-webui/server/src/main.ts:8-8
Timestamp: 2024-12-31T19:19:55.032Z
Learning: When "moduleResolution": "node16" is used in a TypeScript project's tsconfig.json, the import file extension is typically expected to be ".js" (or ".cjs"/".mjs"), even if the source code is originally in TypeScript (.ts). This is a part of node16’s resolution behavior.
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
components/webui/common/tsconfig.json (5)
Learnt from: junhaoliao
PR: y-scope/clp#647
File: components/log-viewer-webui/server/src/main.ts:8-8
Timestamp: 2024-12-31T19:19:55.032Z
Learning: When "moduleResolution": "node16" is used in a TypeScript project's tsconfig.json, the import file extension is typically expected to be ".js" (or ".cjs"/".mjs"), even if the source code is originally in TypeScript (.ts). This is a part of node16’s resolution behavior.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: y-scope/clp#949
File: components/log-viewer-webui/client/src/components/QueryBox/index.tsx:15-15
Timestamp: 2025-05-30T20:43:38.325Z
Learning: In TypeScript/JavaScript files, exports should be placed at the end of the source file rather than inline with declarations. Prefer pattern like `interface SomeInterface {...}` followed by `export {SomeInterface};` at the end rather than `export interface SomeInterface {...}` at the declaration site.
Learnt from: junhaoliao
PR: y-scope/clp#949
File: components/log-viewer-webui/client/src/components/QueryBox/index.tsx:15-15
Timestamp: 2025-05-30T20:43:38.325Z
Learning: In TypeScript/JavaScript files, exports should be placed at the end of the source file rather than inline with declarations. Prefer pattern like `interface SomeInterface {...}` followed by `export {SomeInterface};` at the end rather than `export interface SomeInterface {...}` at the declaration site.
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
components/webui/client/src/components/SqlEditor/monaco-sql.d.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/server/package.json (3)
Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
components/webui/client/src/components/SqlEditor/index.tsx (3)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: gibber9809
PR: y-scope/clp#672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
components/webui/server/src/plugins/app/socket/MongoSocketIoServer/typings.ts (3)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:32-38
Timestamp: 2025-05-09T18:05:42.814Z
Learning: When implementing Socket.IO namespaces or connection pooling in a client-server architecture (like the MongoDB collection socket system in the log-viewer-webui), coordinated changes are needed on both client and server sides, making it appropriate to track as a TODO rather than implement piecemeal.
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
taskfile.yaml (5)
Learnt from: anlowee
PR: y-scope/clp#925
File: taskfiles/deps/main.yaml:97-106
Timestamp: 2025-05-28T18:33:30.155Z
Learning: In the taskfiles dependency system (taskfiles/deps/main.yaml), echo commands are used to generate .cmake settings files that are consumed by the main CMake build process. These files set variables like ANTLR_RUNTIME_HEADER to point to dependency locations for use during compilation.
Learnt from: kirkrodrigues
PR: y-scope/clp#881
File: components/core/tools/scripts/lib_install/macos/install-all.sh:15-23
Timestamp: 2025-05-06T09:46:42.639Z
Learning: For temporary solutions in installation scripts like those in `components/core/tools/scripts/lib_install/`, checksumming downloaded files is considered optional, particularly when working with trusted sources like GitHub raw content.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.
components/webui/package.json (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🧬 Code Graph Analysis (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
components/clp-package-utils/clp_package_utils/general.py (1)
get_clp_home(99-115)
🔇 Additional comments (16)
components/webui/client/public/settings.json (1)
3-3: Ignore enum value casing suggestion for ClpQueryEngineThe
CLP_QUERY_ENGINESenum definesPRESTO = "presto"so the lowercase string in
settings.jsonalready matches the enum’s literal value. No change is needed.Likely an incorrect or invalid review comment.
components/webui/server/src/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts (1)
7-8: Migration import looks good.Switching to the scoped
@webui/commonpackage eliminates brittle relative paths and improves maintainability.components/webui/server/src/routes/api/search/utils.ts (1)
3-4: Confirm tree-shakable re-export.If
SEARCH_SIGNALis re-exported withexport *, ensure the new@webui/commonbuild preserves enum semantics (ESM/TS → CJS). Otherwise runtimeundefinedissues may surface.components/webui/server/src/routes/api/search/typings.ts (1)
10-14: Export leakage risk.Because
SearchResultsMetadataDocumentis re-exported from this file (see lines 45-51), importing it again from@webui/commoncan create two distinct types in downstream code if the compiler resolves them differently (path-based identity). Consider re-exporting the upstream symbol instead of redeclaring:export type { SearchResultsMetadataDocument } from "@webui/common";⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#949 File: components/log-viewer-webui/client/src/components/QueryBox/index.tsx:15-15 Timestamp: 2025-05-30T20:43:38.325Z Learning: In TypeScript/JavaScript files, exports should be placed at the end of the source file rather than inline with declarations. Prefer pattern like `interface SomeInterface {...}` followed by `export {SomeInterface};` at the end rather than `export interface SomeInterface {...}` at the declaration site.Learnt from: junhaoliao PR: y-scope/clp#949 File: components/log-viewer-webui/client/src/components/QueryBox/index.tsx:15-15 Timestamp: 2025-05-30T20:43:38.325Z Learning: In TypeScript/JavaScript files, exports should be placed at the end of the source file rather than inline with declarations. Prefer pattern like `interface SomeInterface {...}` followed by `export {SomeInterface};` at the end rather than `export interface SomeInterface {...}` at the declaration site.Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:16-23 Timestamp: 2024-11-21T15:51:33.203Z Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.Learnt from: haiqi96 PR: y-scope/clp#620 File: components/core/src/clp/clo/OutputHandler.cpp:103-103 Timestamp: 2024-12-02T22:35:37.758Z Learning: In `OutputHandler.cpp`, the field `cResultsCacheKeys::SearchOutput::OrigFileId` is intentionally used for search output metadata and is not related to extracted stream metadata. It should not be renamed to `StreamId`.components/webui/server/src/routes/api/search/index.ts (1)
7-11: Good alias usage — double-check tsconfig paths.The new
@webui/commonimport cleans up deep relative paths; just verifypaths/exportsare wired so that both server (ts-node) and build output resolve the package the same way.components/webui/client/src/api/socket/MongoSocketCursor.ts (1)
1-7: Client import switched to shared package—remember to bump client deps.
@webui/commonmust be declared inclient/package.jsonand locked to the same version as the server to avoid mismatched generated socket types at runtime.components/webui/server/src/plugins/app/socket/MongoSocketIoServer/typings.ts (1)
14-14: LGTM! Clean import path refactoring.The change from a deep relative import to the new
@webui/commonpackage alias improves maintainability and aligns with the package restructuring.components/webui/server/src/plugins/app/socket/MongoSocketIoServer/index.ts (1)
24-24: LGTM! Consistent import path update.The import path change to
@webui/commonmaintains consistency with the package restructuring and improves code organization.components/webui/client/package.json (1)
24-24: LGTM! Package restructuring dependency.The
@webui/commondependency properly supports the package restructuring and centralization of shared types.components/webui/server/src/routes/static.ts (1)
18-18: rootDirname correctly resolves to webui root
The calculationpath.resolve(__dirname, '../../..')from
components/webui/server/src/routes/static.tsclimbs three levels tocomponents/webui, matching the actual directory layout. Static assets such asclient/publicand other sub-directories live under this path, so no adjustment is needed.components/webui/common/tsconfig.json (1)
1-12: LGTM! Standard TypeScript configuration for the common package.The configuration is appropriate for a modern Node.js TypeScript package, using current best practices with ES2022 target, Node.js native modules, and strict type checking.
components/webui/server/package.json (2)
35-35: Dependency addition looks correct
@webui/commonis added and version-pinned. No further issues spotted.
9-12: Confirm build output aligns with npm script paths
Yourtsconfig.jsonsets"outDir": "dist"(components/webui/server/tsconfig.json:13), so TypeScript will emit files underdist, preserving your inferredrootDir. Verify that after a clean build you have:
dist/src/app.jsdist/src/main.jsIf those files don’t exist, either:
- Explicitly set
"rootDir": "src"or change"outDir"to"dist/src"in components/webui/server/tsconfig.json- Or update the paths in the scripts block of components/webui/server/package.json (lines 9–12) to point to
dist/app.jsanddist/main.jsThis ensures you won’t hit
MODULE_NOT_FOUNDerrors at runtime.components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
979-982: No stale references to the old entry-point found
A search fordist/server/src/main.jsacross the codebase returned no matches. No further updates are required.components/webui/package.json (2)
5-9: Workspace declaration looks good
common,client, andserverare correctly listed; this enables deterministic installations with onenpm clean-install.
25-25: Major-version bump ofconcurrently– confirm no breaking flag changes
concurrentlyv9 introduced minor CLI breaking changes (e.g., default quoting behaviour). Verify thatnpm run startstill launches both workspaces correctly.
components/webui/server/src/plugins/app/socket/MongoSocketIoServer/MongoWatcherCollection.ts
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
Outdated
Show resolved
Hide resolved
| self.MonacoEnvironment = { | ||
| getWorker(_, label) { | ||
| if (label === 'json') { | ||
| return new jsonWorker(); | ||
| } | ||
| if (label === 'css' || label === 'scss' || label === 'less') { | ||
| return new cssWorker(); | ||
| } | ||
| if (label === 'html' || label === 'handlebars' || label === 'razor') { | ||
| return new htmlWorker(); | ||
| } | ||
| if (label === 'typescript' || label === 'javascript') { | ||
| return new tsWorker(); | ||
| } | ||
| return new editorWorker(); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
self.MonacoEnvironment assignment breaks TypeScript strict checks
MonacoEnvironment isn’t declared on globalThis; with strict or noImplicitAny the file fails to compile.
Apply the cast to avoid the compile-time error:
-self.MonacoEnvironment = {
+(self as any).MonacoEnvironment = {For stronger typing you could extend the global scope:
declare global {
interface Window {
MonacoEnvironment?: { getWorker(_: any, label: string): Worker };
}
}🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlEditor/monaco-config.ts around
lines 10 to 26, the assignment to self.MonacoEnvironment causes TypeScript
strict compilation errors because MonacoEnvironment is not declared on
globalThis. To fix this, cast self to any or extend the global Window interface
by declaring MonacoEnvironment with the correct type signature for getWorker.
This will satisfy TypeScript's strict type checks and prevent compile errors.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts (1)
6-6: TypeScript strict mode compatibility issueThe assignment to
self.MonacoEnvironmentcauses TypeScript strict compilation errors becauseMonacoEnvironmentis not declared onglobalThis.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts (1)
Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
🪛 GitHub Actions: clp-lint
components/webui/client/src/components/SqlEditor/monaco-config.ts
[error] 3-9: ESLint errors: Expected 2 empty lines after import statement (import/newline-after-import); Expected indentation of 4 spaces but found 2 (@stylistic/indent); Missing space before function parentheses (@stylistic/space-before-function-paren); Expected indentation of 8 spaces but found 4 (@stylistic/indent); Expected indentation of 4 spaces but found 2 (@stylistic/indent).
[warning] 3-7: ESLint warnings: Strings must use doublequote (@stylistic/quotes); Missing JSDoc comment (jsdoc/require-jsdoc).
🔇 Additional comments (2)
components/webui/client/src/components/SqlEditor/monaco-config.ts (2)
12-12: LGTM!The loader configuration is correctly implemented.
6-10: Fix indentation and formatting violations.The code has multiple formatting issues that need to be addressed:
+/** + * Configure Monaco Editor environment for web workers. + */ -self.MonacoEnvironment = { - getWorker() { - return new EditorWorker(); - }, -}; +self.MonacoEnvironment = { + getWorker () { + return new EditorWorker(); + }, +};Likely an incorrect or invalid review comment.
components/webui/client/src/components/SqlEditor/monaco-config.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts (1)
10-19:self.MonacoEnvironmentstill breaks strict TS – apply a cast or extendglobalThis.
The assignment triggersProperty 'MonacoEnvironment' does not exist on type 'WorkerGlobalScope'understrictornoImplicitAny. The same issue was raised earlier and remains unresolved.- self.MonacoEnvironment = { + (self as any).MonacoEnvironment = { getWorker() { return new EditorWorker(); }, };For safer typing, declare the interface globally instead of using
any.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/components/SqlEditor/monaco-config.ts
🧠 Learnings (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts (3)
Learnt from: junhaoliao
PR: #596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In components/log-viewer-webui/client/src/api/query.js, the ExtractJsonResp type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.
Learnt from: junhaoliao
PR: #1078
File: components/webui/server/src/plugins/MongoSocketIoServer/index.ts:105-108
Timestamp: 2025-07-08T17:20:02.560Z
Learning: The y-scope/clp codebase has a 100-character line length limit that takes precedence over other style preferences like template literals when they conflict.
Learnt from: anlowee
PR: #925
File: components/core/src/clp_s/search/kql/antlr_generated/KqlLexer.cpp:8-13
Timestamp: 2025-05-26T18:41:26.096Z
Learning: Do not suggest modifications to auto-generated code files, even for minor issues like duplicate declarations or formatting inconsistencies.
🔇 Additional comments (1)
components/webui/client/src/components/SqlEditor/monaco-config.ts (1)
1-1:/* eslint-disable import/default */appears unnecessary.
No default import is used; remove the blanket disable and let ESLint enforce correct import style.⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#937 File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24 Timestamp: 2025-05-29T20:33:40.653Z Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.Learnt from: davemarco PR: y-scope/clp#797 File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5 Timestamp: 2025-04-08T22:32:05.366Z Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.Learnt from: junhaoliao PR: y-scope/clp#596 File: components/log-viewer-webui/client/src/api/query.js:35-41 Timestamp: 2024-11-19T19:52:43.429Z Learning: For internal APIs in `components/log-viewer-webui/client/src/api/query.js`, runtime validation of parameters may not be necessary since the APIs are not exposed to end users, and JsDoc type annotations may be sufficient.Learnt from: davemarco PR: y-scope/clp#797 File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5 Timestamp: 2025-04-08T22:32:05.367Z Learning: React Router v7.4.1 uses a different package structure than previous versions. Components can be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions. Alternatively, DOM-specific components can also be imported using deep imports like `import { Link } from "react-router/dom";`.
| import "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js"; | ||
| import "monaco-editor/esm/vs/editor/editor.all.js"; | ||
|
|
There was a problem hiding this comment.
🛠️ 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.
There was a problem hiding this comment.
the autocomplete dosent work without it
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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!
components/webui/client/src/components/SqlEditor/monaco-config.ts
Outdated
Show resolved
Hide resolved
| /* 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"; |
There was a problem hiding this comment.
we can import only what we need, which saves another half MB:
| /* 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 */ |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
🧩 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.
| import "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js"; | ||
| import "monaco-editor/esm/vs/editor/editor.all.js"; | ||
|
|
| @@ -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", | |||
There was a problem hiding this comment.
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.
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
the suggestion pop shows up for any input that matches the SQL keyword prefixes. e.g.,
SELECT a ->
ABORTABSOLUTE- ...
and hitting "Enter" accepts the very first suggestion.
since it's common to have multiple lines in a SQL query (i.e., users might hit "Enter" very frequently) and that users might not be used to typing s (spaces) to dismiss the suggestions, can we add the current user input as a first suggestion? That way, existing user inputs are preserved and users can select the suggested keywords with arrow keys only if they wish
|
@hoophalab all yours again - |
components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
Outdated
Show resolved
Hide resolved
hoophalab
changed the title
Description
PR adds the monaco editor to the webui for SQL input, and adds support for highlighting and autocomplete for sql syntax like
SELECT,WHEREetc.I also added a "run" button, which seemed more appropriate than "search" since we support more queries with sql.
They only render when presto is set as query engine in settings.
Checklist
breaking change.
Validation performed
Editor loads. To validate yourself, you must change from ClpQueryEngine from native to presto in settings.json in public folder.
Summary by CodeRabbit
New Features
Enhancements
Chores