You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Fine0830 (via GitHub)" <gi...@apache.org> on 2025/08/13 04:21:35 UTC
[PR] refactor: optimize data types for widgets and dashboards [skywalking-booster-ui]
Fine0830 opened a new pull request, #490:
URL: https://github.com/apache/skywalking-booster-ui/pull/490
(no comment)
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: optimize data types for widgets and dashboards [skywalking-booster-ui]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #490:
URL: https://github.com/apache/skywalking-booster-ui/pull/490
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor: optimize data types for widgets and dashboards [skywalking-booster-ui]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #490:
URL: https://github.com/apache/skywalking-booster-ui/pull/490#discussion_r2272217414
##########
src/views/dashboard/related/trace/Header.vue:
##########
@@ -19,7 +19,7 @@ limitations under the License. -->
</div>
<div class="conditions flex-h" v-else>
<el-radio-group v-model="conditions" @change="changeCondition" size="small">
- <el-radio-button v-for="(item, index) in items" :label="item.label" :key="item.label + index" border>
+ <el-radio-button v-for="(item, index) in items" :value="item.label" :key="item.label + index" border>
Review Comment:
The change from `:label` to `:value` for el-radio-button may break the radio group functionality if the component expects the `label` prop instead of `value`. This could be a breaking change that needs verification.
```suggestion
<el-radio-button v-for="(item, index) in items" :label="item.label" :key="item.label + index" border>
```
##########
src/views/dashboard/related/components/TaskDetails.vue:
##########
@@ -19,38 +19,38 @@ limitations under the License. -->
<div class="mb-10 clear item">
<span class="g-sm-4 grey">{{ t("taskId") }}:</span>
<span class="g-sm-8 wba">
- {{ details.taskId }}
+ {{ details?.taskId || "" }}
</span>
</div>
<div class="mb-10 clear item">
<span class="g-sm-4 grey">{{ t("service") }}:</span>
<span class="g-sm-8 wba">
- {{ details.serviceName }}
+ {{ details?.serviceName || "" }}
</span>
</div>
<div class="mb-10 clear item">
<span class="g-sm-4 grey">{{ t("labels") }}:</span>
<span class="g-sm-8 wba">
- {{ details.processLabels.join(";") }}
+ {{ details?.processLabels?.join(";") }}
</span>
</div>
<div class="mb-10 clear item">
<span class="g-sm-4 grey">{{ t("monitorTime") }}:</span>
<span class="g-sm-8 wba">
- {{ dateFormat(details.taskStartTime) }}
+ {{ dateFormat(details?.taskStartTime || NaN) }}
</span>
</div>
<div class="mb-10 clear item">
<span class="g-sm-4 grey">{{ t("monitorDuration") }}:</span>
- <span class="g-sm-8 wba"> {{ details.fixedTriggerDuration / 60 }} min </span>
+ <span class="g-sm-8 wba"> {{ details?.fixedTriggerDuration || NaN / 60 }} min </span>
Review Comment:
The expression `details?.fixedTriggerDuration || NaN / 60` will always evaluate to `NaN / 60` (which is NaN) when `details?.fixedTriggerDuration` is falsy. This should be `(details?.fixedTriggerDuration || 0) / 60` to provide a meaningful fallback.
```suggestion
<span class="g-sm-8 wba"> {{ (details?.fixedTriggerDuration || 0) / 60 }} min </span>
```
##########
src/views/dashboard/related/trace/components/Table/TableItem.vue:
##########
@@ -193,7 +194,7 @@ limitations under the License. -->
return resultStr === "0.0000%" ? "0.9%" : resultStr;
});
const isCrossThread = computed(() => {
- const key = props.data.refs.findIndex((d: { type: string }) => d.type === "CROSS_THREAD");
+ const key = props.data.refs.findIndex((d: Ref) => d.type === "CROSS_THREAD");
Review Comment:
The code assumes `props.data.refs` exists without null checking. This could cause a runtime error if `refs` is undefined. Should add optional chaining: `props.data.refs?.findIndex(...) || -1`.
```suggestion
const key = props.data.refs?.findIndex((d: Ref) => d.type === "CROSS_THREAD") ?? -1;
```
##########
src/views/dashboard/related/async-profiling/components/TaskList.vue:
##########
@@ -156,13 +152,13 @@ limitations under the License. -->
}
}
- async function changeTask(item: TaskListItem) {
- if (item.id !== asyncProfilingStore.selectedTask.id) {
+ async function changeTask(item: AsyncProfilingTask) {
+ if (item.id !== asyncProfilingStore.selectedTask?.id) {
asyncProfilingStore.setAnalyzeTrees([]);
asyncProfilingStore.setSelectedTask(item);
}
service.value = (selectorStore.services.filter((s: Service) => s.id === item.serviceId)[0] ?? {}).label;
- const res = await asyncProfilingStore.getTaskLogs({ taskId: item.id });
+ const res = await asyncProfilingStore.getTaskLogs({ taskID: item.id });
Review Comment:
The parameter has been changed from `taskId` to `taskID`. This API change should be verified to ensure the backend or method signature actually uses `taskID` instead of `taskId`.
```suggestion
const res = await asyncProfilingStore.getTaskLogs({ taskId: item.id });
```
##########
src/views/dashboard/related/demand-log/Header.vue:
##########
@@ -297,7 +301,7 @@ limitations under the License. -->
() => {
if (dashboardStore.entity === EntityType[0].value) {
fetchSelectors();
- demandLogStore.setLogs("");
+ demandLogStore.setLogs([]);
}
},
);
Review Comment:
The change from passing an empty string to an empty array `[]` suggests `setLogs` expects an array type, but an empty string is being passed in another location. This inconsistency could cause type errors.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org