Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
367 changes: 172 additions & 195 deletions components/log-viewer-webui/client/package-lock.json

Large diffs are not rendered by default.

11 changes: 9 additions & 2 deletions components/log-viewer-webui/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"build": "tsc -b && vite build",
"lint:check": "eslint",
"lint:fix": "eslint --fix",
"start": "vite",
"start": "vite --host 10.1.0.21",
Comment thread
AVMatthews marked this conversation as resolved.
Resolved
"antd": "VITE_USE_ANTD_APP=true npm run start"
},
"author": "YScope Inc. <dev@yscope.com>",
Expand All @@ -21,13 +21,20 @@
"antd": "^5.24.5",
"axios": "^1.7.9",
"react": "^19.0.0",
"react-dom": "^19.0.0"
"react-dom": "^19.0.0",
"socket.io-client": "^4.8.1"
},
"devDependencies": {
"@eslint/js": "^9.19.0",
"@types/react": "^19.0.10",
"@types/react-dom": "^19.0.4",
"@vitejs/plugin-react": "^4.3.4",
"eslint": "^9.19.0",
"eslint-config-yscope": "latest",
"eslint-plugin-react": "^7.37.4",
"eslint-plugin-react-hooks": "^5.0.0",
"eslint-plugin-react-refresh": "^0.4.18",
"globals": "^15.14.0",
Comment on lines +28 to +38

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't have to explicitly state those eslint dependencies because eslint-config-yscope lists those as "peer" dependencies, so npm install will install those even if not explicitly specified.

Suggested change
"@eslint/js": "^9.19.0",
"@types/react": "^19.0.10",
"@types/react-dom": "^19.0.4",
"@vitejs/plugin-react": "^4.3.4",
"eslint": "^9.19.0",
"eslint-config-yscope": "latest",
"eslint-plugin-react": "^7.37.4",
"eslint-plugin-react-hooks": "^5.0.0",
"eslint-plugin-react-refresh": "^0.4.18",
"globals": "^15.14.0",
"@types/react": "^19.0.10",
"@types/react-dom": "^19.0.4",
"@vitejs/plugin-react": "^4.3.4",
"eslint-config-yscope": "latest"

"typescript": "~5.6.2",
"vite": "^6.2.5"
}
Expand Down
37 changes: 31 additions & 6 deletions components/log-viewer-webui/client/src/App.tsx

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NOTE: changes in this file is to be removed.

Original file line number Diff line number Diff line change
@@ -1,19 +1,44 @@
import {CssVarsProvider} from "@mui/joy";
// import { CssVarsProvider } from "@mui/joy";
// import { LOCAL_STORAGE_KEY } from "./typings/config";
// import QueryStatus from "./ui/QueryStatus";
import MongoReplicaCollection from "./mongoCDCLib/MongoReplicaCollection";
import {useTracker} from "./mongoCDCLib/useTracker";

import {LOCAL_STORAGE_KEY} from "./typings/config";
import QueryStatus from "./ui/QueryStatus";

const collection = new MongoReplicaCollection("results-1");

/**
* Renders the main application.
*
* @return
*/
const App = () => {
const results = useTracker(
() => collection.find({}, {}),
[]
);

console.log("results len:", results.length);
for (let i = 0; i < results.length; i++) {
console.log(results[i]);
}

// const results = ['123'];
// results = ["123"];

return (
<CssVarsProvider modeStorageKey={LOCAL_STORAGE_KEY.THEME}>
<QueryStatus/>
</CssVarsProvider>
<div>
{results.map((r) => (
<div>
{JSON.stringify(r)}
</div>
))}

</div>

// <CssVarsProvider modeStorageKey={LOCAL_STORAGE_KEY.THEME}>
// <QueryStatus/>
// </CssVarsProvider>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {
io,
Socket,
} from "socket.io-client";

import MongoReplicaCollectionReactiveCursor from "./MongoReplicaCollectionReactiveCursor.js";


class MongoReplicaCollection {
private socket: Socket;

constructor (collectionName: string) {
// TODO: use the server URL from the environment / constructor args
this.socket = io();

this.socket.emit("collection::init", {
collectionName: collectionName,
});
}

// TODO: add support for non-reactive cursors
find (query: object, options: object) {
return new MongoReplicaCollectionReactiveCursor({
options: options,
query: query,
socket: this.socket,
});
}
}

// const main = async () => {
// const collection = new MongoReplicaCollection("3");
// const cursor = collection.find({}, {});

// console.log(await cursor.toArray());
// };

//
// await main();

export default MongoReplicaCollection;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we tend to reserve two empty lines before exports. Sorry about that the linter has not been smart enough to autofix this.

Suggested change
export default MongoReplicaCollection;
export default MongoReplicaCollection;

Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { Socket } from "socket.io-client";

interface CursorConstructorArgs {
socket: Socket;
query: object;
options: object;
}

class MongoReplicaCollectionCursor {
socket: Socket;

findQuery: object;

findOptions: object;

constructor ({socket, query, options}: CursorConstructorArgs) {
this.socket = socket;

this.findQuery = query;
this.findOptions = options;
}

sort (sort: object): this {
this.findOptions = {
...this.findOptions,
sort: sort,
};

return this;
}

limit (number: number): this {
this.findOptions = {
...this.findOptions,
limit: number,
};

return this;
}

skip (offset:number): this {
this.findOptions = {
...this.findOptions,
skip: offset,
};

return this;
}

toArray () {
return new Promise((resolve, reject) => {
this.socket.emit("collection::find::toArray", {
query: this.findQuery,
options: this.findOptions,
}, (response: { error?: any; data?: any }) => {
if (response.error) {
return reject(response.error);
}

return resolve(response.data);
});
});
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NOTE: not in use at the moment.



export default MongoReplicaCollectionCursor;
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import MongoReplicaCollectionCursor from "./MongoReplicaCollectionCursor.js";
import { Socket } from "socket.io-client";

interface Listener {
onData: (data: any) => void;
onError: (error: any) => void;
}

interface ReactiveArrayCallback {
onData: (data: any) => void;
onError: (error: any) => void;
}

class MongoReplicaCollectionReactiveCursor extends MongoReplicaCollectionCursor {
/**
* @type {Listener}
*/
private listener: Listener | null = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For private properties / functions, can we use the #propertyName syntax?


constructor (props: {socket: Socket; query: object; options: object}) {
super(props);

this.socket.on("collection::find::update", (response: {error?: any; data?: any}) => {
if (response.error) {
return this.listener?.onError(response.error);
}

return this.listener?.onData(response.data);
});
}

/**
* Subscribe to the collection
*
* @param {{onData: Function, onError: Function}} callback
* @return {function(): void} The cleanup function.
*/
toReactiveArray (callback: ReactiveArrayCallback): () => void {
console.log("toReactiveArrya");
let queryHash: string | undefined;
this.socket.emit("collection::find::toReactiveArray", {
query: this.findQuery,
options: this.findOptions,
}, (response: {error?: any; queryHash?: string}) => {
if (response.error) {
return callback.onError(response.error);
}

({queryHash} = response);

this.listener = callback;
});

return () => {
// TODO: the server should also rely on some heartbeat from the client
// for deciding when to free the resources.
this.socket.emit("collection::find::unsubscribe", {
queryHash: queryHash,
});

this.socket.off("collection::find::update");
};
}
}

export default MongoReplicaCollectionReactiveCursor;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
export default MongoReplicaCollectionReactiveCursor;
export default MongoReplicaCollectionReactiveCursor;

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import {
useEffect,
useState,
} from "react";

/**
* Custom hook to track a MongoDB cursor and return its data as an array.
*
* @param {Function} getCursor Function that returns a MongoReplicaCollectionReactiveCursor.
* @param {object[]} dependencies Array of dependencies for the effect.
* @return {object[]} The data from the cursor as an array.
*/
const useTracker = (
getCursor: Function,
dependencies: object[] = []
): any[] => {
const [data, setData] = useState<any[]>([]);

useEffect(() => {
const cursor = getCursor();

const handleData = (newData: any[]) => setData(newData);

const unsubscribe = cursor.toReactiveArray({
onData: handleData,
onError: (error: any) => {
console.error("Error fetching data:", error);
},
});

return () => {
unsubscribe();
};
// eslint-disable-next-line react-hooks/exhaustive-deps

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// eslint-disable-next-line react-hooks/exhaustive-deps
// `getCursor` is a function that's intentionally excluded from the dependencies array. Including it would
// cause unnecessary re-subscriptions or even infinite loops if it's redefined on each render.
// eslint-disable-next-line react-hooks/exhaustive-deps

}, dependencies);

return data;
};

export {useTracker};
Comment on lines +44 to +45

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's do default export here:

Suggested change
export {useTracker};
export default useTracker;

We also need to update the import in the sample App.tsx:

import useTracker from ...;

5 changes: 5 additions & 0 deletions components/log-viewer-webui/client/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export default defineConfig({
target: "http://localhost:3000/",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the comment in L12-13 applies to both routes:

  1. Let move the comment to Line 11
  2. Let say "target host and port" instead of "target" to improve clarity.

@coderabbitai Let's create an issue to track the effort of serving Fastify routes under "/api". And we should replace this "/query/" proxy entry with "/api/" to ensure the other RESTful APIs are proxied to the backend.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@junhaoliao I'll create an issue to track this effort. Let me take care of that for you.


🏁 Script executed:

#!/bin/bash

# Create a GitHub issue to track the API routing changes
gh issue create \
  --title "Replace '/query/' proxy with '/api/' for RESTful endpoints" \
  --body "## Background
Currently, the log-viewer-webui client proxies API requests through the '/query/' path, but this doesn't account for other RESTful endpoints.

## Objective
- Serve all Fastify routes under the '/api/' path
- Update the Vite configuration to proxy '/api/' to the backend server instead of just '/query/'

## References
- Requested in PR: https://github.com/y-scope/clp/pull/809
- Discussion comment: https://github.com/y-scope/clp/pull/809#discussion_r2034462565

## Assignee
@junhaoliao" \
  --assignee junhaoliao \
  --repo y-scope/clp

Length of output: 718


@junhaoliao I've created an issue to track the effort of serving Fastify routes under "/api/" and replacing the "/query/" proxy entry. You can find it here: #810

The issue includes the objectives we discussed about updating the Vite configuration to ensure all RESTful APIs are properly proxied to the backend.

changeOrigin: true,
},
"/socket.io/": {
target: "ws://localhost:3000/",
changeOrigin: true,
ws: true,
},
},
},
publicDir: "public",
Expand Down
Loading