Copilot commented on code in PR #845:
URL:
https://github.com/apache/skywalking-banyandb/pull/845#discussion_r2516471350
##########
ui/src/api/index.js:
##########
@@ -17,216 +17,334 @@
* under the License.
*/
+import { queryClient } from '@/plugins/vue-query';
import { httpQuery } from './base';
+function fetchWithQuery(queryKey, request, options = {}) {
+ return queryClient.fetchQuery({
+ queryKey,
+ queryFn: () => httpQuery(request),
+ ...options,
+ });
+}
+
+async function invalidateQueries(keys = []) {
+ if (!Array.isArray(keys) || keys.length === 0) {
+ return;
+ }
+
+ await Promise.all(
+ keys.map((key) => {
+ if (typeof key === 'function') {
+ return queryClient.invalidateQueries({ predicate: key });
+ }
+ if (key && typeof key === 'object' && key.queryKey) {
+ return queryClient.invalidateQueries(key);
+ }
+ const queryKey = Array.isArray(key) ? key : [key];
+ return queryClient.invalidateQueries({ queryKey });
+ }),
+ );
+}
+
+async function mutateWithInvalidation(request, invalidate = []) {
+ const result = await httpQuery(request);
+ if (!result?.error && invalidate.length) {
+ await invalidateQueries(invalidate);
+ }
+ return result;
+}
Review Comment:
The `mutateWithInvalidation` function lacks documentation. Add JSDoc to
explain its purpose as a mutation wrapper, the invalidation strategy, and the
expected structure of the `invalidate` parameter.
##########
ui/src/api/index.js:
##########
@@ -17,216 +17,334 @@
* under the License.
*/
+import { queryClient } from '@/plugins/vue-query';
import { httpQuery } from './base';
+function fetchWithQuery(queryKey, request, options = {}) {
+ return queryClient.fetchQuery({
+ queryKey,
+ queryFn: () => httpQuery(request),
+ ...options,
+ });
+}
+
+async function invalidateQueries(keys = []) {
+ if (!Array.isArray(keys) || keys.length === 0) {
+ return;
+ }
+
+ await Promise.all(
+ keys.map((key) => {
+ if (typeof key === 'function') {
+ return queryClient.invalidateQueries({ predicate: key });
+ }
+ if (key && typeof key === 'object' && key.queryKey) {
+ return queryClient.invalidateQueries(key);
+ }
+ const queryKey = Array.isArray(key) ? key : [key];
+ return queryClient.invalidateQueries({ queryKey });
+ }),
+ );
+}
+
+async function mutateWithInvalidation(request, invalidate = []) {
+ const result = await httpQuery(request);
+ if (!result?.error && invalidate.length) {
+ await invalidateQueries(invalidate);
+ }
+ return result;
+}
+
export function getGroupList() {
- return httpQuery({
+ return fetchWithQuery(['groups'], {
url: '/api/v1/group/schema/lists',
method: 'GET',
});
}
export function getAllTypesOfResourceList(type, name) {
- return httpQuery({
+ return fetchWithQuery(['groupResources', type, name], {
url: `/api/v1/${type}/schema/lists/${name}`,
method: 'GET',
});
}
export function getResourceOfAllType(type, group, name) {
- return httpQuery({
+ return fetchWithQuery(['resource', type, group, name], {
url: `/api/v1/${type}/schema/${group}/${name}`,
method: 'GET',
});
}
export function getTableList(data, type) {
- return httpQuery({
- url: `/api/v1/${type}/data`,
- json: data,
- method: 'POST',
- });
+ const queryKey = ['tableData', type, JSON.stringify(data)];
+ return fetchWithQuery(
+ queryKey,
+ {
+ url: `/api/v1/${type}/data`,
+ json: data,
+ method: 'POST',
+ },
+ { staleTime: 0 },
Review Comment:
Setting `staleTime: 0` for POST request queries (getTableList,
getTopNAggregationData, fetchProperties, queryTraces, executeBydbQLQuery)
disables caching entirely, defeating the purpose of TanStack Query. If these
queries should not be cached due to dynamic data, consider using `useMutation`
instead of `useQuery`, or use a very short staleTime (e.g., 1000ms) to allow
deduplication of simultaneous requests while preventing stale data.
```suggestion
{ staleTime: 1000 },
```
##########
ui/vite.config.mjs:
##########
@@ -40,15 +43,64 @@ export default ({ mode }) => {
resolve: {
alias: {
'@': fileURLToPath(new URL('./src', import.meta.url)),
+ 'vue-demi': 'vue-demi/lib/index.mjs',
},
},
+ optimizeDeps: {
+ include: [
+ '@tanstack/vue-query',
+ 'codemirror',
+ 'echarts',
+ 'element-plus/es',
+ 'vue-demi',
+ 'vue-router',
+ ],
+ },
css: {
preprocessorOptions: {
scss: {
api: 'modern-compiler',
},
},
},
+ esbuild: {
+ drop: isProduction ? ['console', 'debugger'] : [],
+ },
+ define: {
+ __VUE_OPTIONS_API__: false,
Review Comment:
Setting `__VUE_OPTIONS_API__: false` disables the Options API entirely. This
is a breaking change if any existing components use the Options API (data(),
methods, computed, etc.). Verify that all components in the codebase use only
the Composition API before disabling this feature.
```suggestion
__VUE_OPTIONS_API__: true,
```
##########
ui/vite.config.mjs:
##########
@@ -40,15 +43,64 @@ export default ({ mode }) => {
resolve: {
alias: {
'@': fileURLToPath(new URL('./src', import.meta.url)),
+ 'vue-demi': 'vue-demi/lib/index.mjs',
},
},
+ optimizeDeps: {
+ include: [
+ '@tanstack/vue-query',
+ 'codemirror',
+ 'echarts',
+ 'element-plus/es',
+ 'vue-demi',
+ 'vue-router',
+ ],
+ },
css: {
preprocessorOptions: {
scss: {
api: 'modern-compiler',
},
},
},
+ esbuild: {
+ drop: isProduction ? ['console', 'debugger'] : [],
Review Comment:
[nitpick] While dropping console statements in production is common,
dropping all console methods (including console.error and console.warn) can
make production debugging difficult. Consider using a more selective approach
like `drop: isProduction ? ['console.log', 'debugger'] : []` to preserve error
logging.
```suggestion
drop: isProduction ? ['console.log', 'debugger'] : [],
```
##########
ui/src/components/BydbQL/Index.vue:
##########
@@ -572,7 +579,7 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`);
:style-active-line="true"
:auto-refresh="true"
:enable-hint="true"
- class="query-input"
+ :class="['query-input', { 'is-hidden': editorLoading }]"
Review Comment:
When hiding the CodeMirror editor with `visibility: hidden`, it's still in
the accessibility tree. Consider adding `aria-hidden='true'` when
`editorLoading` is true to prevent screen readers from announcing hidden
content, or use `v-show` on both the skeleton and editor to completely remove
from DOM.
##########
ui/src/api/index.js:
##########
@@ -17,216 +17,334 @@
* under the License.
*/
+import { queryClient } from '@/plugins/vue-query';
import { httpQuery } from './base';
+function fetchWithQuery(queryKey, request, options = {}) {
+ return queryClient.fetchQuery({
+ queryKey,
+ queryFn: () => httpQuery(request),
+ ...options,
+ });
+}
Review Comment:
The `fetchWithQuery` function lacks documentation. Consider adding a JSDoc
comment explaining its purpose, parameters (queryKey format, request object
structure), return value, and how it integrates with TanStack Query's caching
mechanism.
##########
ui/src/api/index.js:
##########
@@ -17,216 +17,334 @@
* under the License.
*/
+import { queryClient } from '@/plugins/vue-query';
import { httpQuery } from './base';
+function fetchWithQuery(queryKey, request, options = {}) {
+ return queryClient.fetchQuery({
+ queryKey,
+ queryFn: () => httpQuery(request),
+ ...options,
+ });
+}
+
+async function invalidateQueries(keys = []) {
+ if (!Array.isArray(keys) || keys.length === 0) {
+ return;
+ }
+
+ await Promise.all(
+ keys.map((key) => {
+ if (typeof key === 'function') {
+ return queryClient.invalidateQueries({ predicate: key });
+ }
+ if (key && typeof key === 'object' && key.queryKey) {
+ return queryClient.invalidateQueries(key);
+ }
+ const queryKey = Array.isArray(key) ? key : [key];
+ return queryClient.invalidateQueries({ queryKey });
+ }),
+ );
+}
Review Comment:
The `invalidateQueries` function lacks documentation explaining the
different key formats it accepts (function predicates, objects with queryKey,
arrays, or single strings). Add JSDoc to clarify the expected input formats and
behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
