chore: upgrade frontend dependencies - #836
Conversation
Co-authored-by: Colin <Colin_XKL@outlook.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideUpgrade admin frontend dependencies and tooling (Vue, Vite, Tailwind, ESLint, Vitest, TypeScript) and adjust app code and config for new versions, including stricter typing, updated Arco behaviors, manual Rollup chunks, and test aliases. Sequence diagram for updated numeric param handling in craft_atom.vuesequenceDiagram
actor User
participant AInputNumber as AInputNumber
participant CraftAtom as CraftAtomComponent
User->>AInputNumber: change value in UI
AInputNumber->>CraftAtom: update:model-value(value)
CraftAtom->>CraftAtom: updateNumberParamValue(param, value)
CraftAtom->>CraftAtom: param.value = value ?? ''
note over AInputNumber,CraftAtom: Initial render
CraftAtom->>CraftAtom: toNumberParamValue(param.value)
CraftAtom-->>AInputNumber: model-value (number | undefined)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Code Review
This pull request upgrades various dependencies (including Vue, Vite, ESLint, and TailwindCSS), migrates to the ESLint flat configuration format, and refactors several Vue components to use TypeScript-based props. The feedback highlights several important issues: the manualChunks configuration in Vite should be updated to correctly group Vue and VueUse core packages; the Node.js engine requirement in package.json needs to be bumped to support the upgraded build tools; and the number-parsing helper functions in craft_atom.vue and TopicAggregatorEditor.vue should explicitly handle null and empty strings to prevent incorrect coercion to 0.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ( | ||
| id.includes('/vue/') || | ||
| id.includes('/vue-router/') || | ||
| id.includes('/pinia/') || | ||
| id.includes('/@vueuse/core/') || | ||
| id.includes('/vue-i18n/') | ||
| ) { | ||
| return 'vue'; | ||
| } |
There was a problem hiding this comment.
The manualChunks matching logic for the vue chunk only checks for /vue/ and /@vueuse/core/. This will miss Vue core runtime packages (which are scoped under /@vue/, such as @vue/runtime-core, @vue/shared, @vue/reactivity) and other VueUse packages (like @vueuse/shared). As a result, these core dependencies will not be grouped into the vue chunk, defeating the purpose of the optimization. Consider updating the matching logic to include /@vue/ and /@vueuse/.
| if ( | |
| id.includes('/vue/') || | |
| id.includes('/vue-router/') || | |
| id.includes('/pinia/') || | |
| id.includes('/@vueuse/core/') || | |
| id.includes('/vue-i18n/') | |
| ) { | |
| return 'vue'; | |
| } | |
| if ( | |
| id.includes('/vue/') || | |
| id.includes('/@vue/') || | |
| id.includes('/vue-router/') || | |
| id.includes('/pinia/') || | |
| id.includes('/@vueuse/') || | |
| id.includes('/vue-i18n/') | |
| ) { | |
| return 'vue'; | |
| } |
| "engines": { | ||
| "node": ">=14.0.0" | ||
| }, |
There was a problem hiding this comment.
The Node.js engine requirement is still set to >=14.0.0. However, the upgraded dependencies (such as ESLint v10 and Vite v8) require much newer Node.js versions (ESLint v10 requires >=18.18.0 or >=20.0.0, and Vite v8/v6 requires >=18.0.0). Keeping the engine requirement at >=14.0.0 will cause installation or runtime failures on older Node.js environments. Please update the engine requirement to a compatible version.
| "engines": { | |
| "node": ">=14.0.0" | |
| }, | |
| "engines": { | |
| "node": ">=18.18.0" | |
| }, |
| const toNumberParamValue = (value: CraftParamValue) => { | ||
| if (typeof value === 'number') return value; | ||
| if (value === '') return undefined; | ||
| const parsed = Number(value); | ||
| return Number.isNaN(parsed) ? undefined : parsed; | ||
| }; |
There was a problem hiding this comment.
In JavaScript, Number(null) evaluates to 0 and Number('') evaluates to 0. While empty string is explicitly handled, null or undefined values are not. If value is null (e.g., from an API response or cleared state), toNumberParamValue will return 0 instead of undefined, causing the input field to display 0 instead of remaining empty. Consider explicitly checking for null and undefined.
const toNumberParamValue = (value: CraftParamValue) => {
if (value === null || value === undefined || value === '') return undefined;
if (typeof value === 'number') return value;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};
| const toNumberValue = (value: string | number) => { | ||
| if (typeof value === 'number') return value; | ||
| const parsed = Number(value); | ||
| return Number.isNaN(parsed) ? undefined : parsed; | ||
| }; |
There was a problem hiding this comment.
In JavaScript, Number('') evaluates to 0. If value is an empty string '' (e.g., when the input is cleared), toNumberValue will return 0 instead of undefined, causing the input field to display 0 instead of remaining empty. Consider explicitly checking for empty string, null, and undefined.
const toNumberValue = (value: string | number | undefined | null) => {
if (value === null || value === undefined || value === '') return undefined;
if (typeof value === 'number') return value;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 1 medium |
🟢 Metrics 18 complexity
Metric Results Complexity 18
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
PR Summary by Qodochore(admin): upgrade frontend deps and migrate lint/build configs
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
Fixed security issues:
-
axios (link)
-
brace-expansion (link)
-
form-data (link)
-
lodash (link)
-
minimatch (link)
-
postcss (link)
-
In
CraftSelector.vueandCraftPickerModal.vue, themodelValueprop is typed asstring | string[]but the default is() => [], which can lead to a runtime type mismatch whenmode === 'single'; consider either making the default''for single mode or changing the prop type to reflect that arrays are always used internally. -
The new ESLint flat config applies
eslint-config-prettierrules globally; if you intend to only disable formatting-related rules for specific file types (e.g., Vue/TS) you may want to scope the Prettier-related rules to those files to avoid unexpected overrides in other parts of the project.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CraftSelector.vue` and `CraftPickerModal.vue`, the `modelValue` prop is typed as `string | string[]` but the default is `() => []`, which can lead to a runtime type mismatch when `mode === 'single'`; consider either making the default `''` for single mode or changing the prop type to reflect that arrays are always used internally.
- The new ESLint flat config applies `eslint-config-prettier` rules globally; if you intend to only disable formatting-related rules for specific file types (e.g., Vue/TS) you may want to scope the Prettier-related rules to those files to avoid unexpected overrides in other parts of the project.
## Individual Comments
### Comment 1
<location path="web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue" line_range="86-90" />
<code_context>
<a-input-number
v-else
- v-model="step.value"
+ :model-value="toNumberValue(step.value)"
:min="1"
mode="button"
style="flex: 1; min-width: 150px"
+ @update:model-value="(value) => updateStepValue(idx, value)"
/>
</code_context>
<issue_to_address>
**suggestion:** Handle empty-string values explicitly in `toNumberValue` to avoid coercing them to `0` unintentionally.
`Number(value)` converts an empty string to `0`, and `updateStepValue` falls back to `1` when `value` is `undefined`, so clearing the input can yield either `0` or `1` depending on timing. To align behavior with `craft_atom.vue` and keep clearing predictable, consider special-casing empty strings:
```ts
const toNumberValue = (value: string | number) => {
if (typeof value === 'number') return value;
if (value === '') return undefined;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};
```
Suggested implementation:
```
const toNumberValue = (value: string | number | undefined) => {
if (typeof value === 'number') return value;
if (value === '') return undefined;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};
```
I assumed there is an existing `toNumberValue` helper defined similarly to `const toNumberValue = (value: string | number) => Number(value);`. If the signature or implementation differs, you should still:
1. Update `toNumberValue` to:
- Accept `string | number | undefined`.
- Return the number directly when given a number.
- Return `undefined` when given an empty string.
- Parse other strings via `Number(...)` and return `undefined` if the result is `NaN`.
2. Ensure `updateStepValue(idx, value)` accepts `value: number | undefined` and keeps the existing behavior of falling back to `1` only when `value` is `undefined`, so that clearing the input consistently maps to the same default as in `craft_atom.vue`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| :model-value="toNumberValue(step.value)" | ||
| :min="1" | ||
| mode="button" | ||
| style="flex: 1; min-width: 150px" | ||
| @update:model-value="(value) => updateStepValue(idx, value)" |
There was a problem hiding this comment.
suggestion: Handle empty-string values explicitly in toNumberValue to avoid coercing them to 0 unintentionally.
Number(value) converts an empty string to 0, and updateStepValue falls back to 1 when value is undefined, so clearing the input can yield either 0 or 1 depending on timing. To align behavior with craft_atom.vue and keep clearing predictable, consider special-casing empty strings:
const toNumberValue = (value: string | number) => {
if (typeof value === 'number') return value;
if (value === '') return undefined;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};Suggested implementation:
const toNumberValue = (value: string | number | undefined) => {
if (typeof value === 'number') return value;
if (value === '') return undefined;
const parsed = Number(value);
return Number.isNaN(parsed) ? undefined : parsed;
};
I assumed there is an existing toNumberValue helper defined similarly to const toNumberValue = (value: string | number) => Number(value);. If the signature or implementation differs, you should still:
- Update
toNumberValueto:- Accept
string | number | undefined. - Return the number directly when given a number.
- Return
undefinedwhen given an empty string. - Parse other strings via
Number(...)and returnundefinedif the result isNaN.
- Accept
- Ensure
updateStepValue(idx, value)acceptsvalue: number | undefinedand keeps the existing behavior of falling back to1only whenvalueisundefined, so that clearing the input consistently maps to the same default as incraft_atom.vue.
WalkthroughThe admin frontend updates its ESLint, TypeScript, Vite, Tailwind, Vitest, and dependency configuration. Vue typings and props use typed forms, while dashboard inputs, forms, event handlers, Axios typing, JSON tree handling, and presentation styles receive targeted updates. ChangesAdmin frontend modernization
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Code Review by Qodo
Context used✅ Compliance rules (platform):
17 rules 1. Bad default modelValue type
|
| const props = withDefaults( | ||
| defineProps<{ | ||
| visible: boolean; | ||
| modelValue: string | string[]; | ||
| mode: 'single' | 'multiple'; | ||
| title?: string; | ||
| }>(), | ||
| { | ||
| visible: false, | ||
| modelValue: () => [], | ||
| mode: 'single', | ||
| title: '', | ||
| } |
There was a problem hiding this comment.
1. Bad default modelvalue type 🐞 Bug ≡ Correctness
CraftPickerModal defaults mode to 'single' but defaults modelValue to [], so when the modal opens the single-mode sync path wraps the array into localValue and can produce a nested array (wrong shape) and incorrect emitted values. The same inconsistent default exists in CraftSelector, so omitting modelValue can pass a non-string into the single-selector path.
Agent Prompt
## Issue description
`CraftPickerModal` and `CraftSelector` default `mode` to `'single'` but default `modelValue` to an array (`[]`). In single mode, `CraftPickerModal` wraps `props.modelValue` into `localValue` on open, and because `[]` is truthy this can create `localValue` with an invalid nested array shape (and then emit wrong values).
## Issue Context
- `CraftPickerModal` uses a `watch(() => props.visible, ...)` to sync `localValue` when opening; the single-mode branch assumes `props.modelValue` is a string.
- `CraftSelector` also defaults `modelValue` to `[]` while defaulting `mode` to `'single'`, and forwards the value as a string via a cast.
## Fix Focus Areas
- web/admin/src/views/dashboard/craft_flow/components/CraftPickerModal.vue[25-66]
- web/admin/src/views/dashboard/craft_flow/CraftSelector.vue[40-53]
## Suggested fix approach
1. Make `modelValue` default consistent with `mode` default:
- Either default `modelValue` to `''` (string) when `mode` defaults to `'single'`, or
- Make `modelValue` optional (default `undefined`) and normalize based on `mode` before using it.
2. Remove reliance on `as string` casts for runtime correctness; instead normalize values explicitly:
- For single mode: coerce arrays to `''`.
- For multiple mode: coerce non-arrays to `[]` (or `[string]` if you want to preserve a string value).
3. Apply the same normalization/default strategy in both `CraftPickerModal` and `CraftSelector` so the API behaves consistently.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-authored-by: Colin <Colin_XKL@outlook.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/admin/src/assets/style/tailwind.css (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint will fail on Tailwind v4's
@config/@importordering — update stylelint config.This ordering is correct per Tailwind v4 (Tailwind requires
@configbefore@importto apply configuration), but the static analysis output shows stylelint flagsno-invalid-position-at-import-ruleandscss/at-rule-no-unknownon these exact lines. If stylelint runs in CI, this file will fail even though it's valid Tailwind v4 usage. Consider adding an override/ignore for these two rules on this file, or switching to a Tailwind-aware stylelint config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/admin/src/assets/style/tailwind.css` around lines 1 - 2, Update the Stylelint configuration for web/admin/src/assets/style/tailwind.css to allow Tailwind v4’s required `@config` before `@import` ordering and recognize the `@config` at-rule. Scope the override to this file where possible, disabling or ignoring no-invalid-position-at-import-rule and scss/at-rule-no-unknown without changing the stylesheet ordering.Source: Linters/SAST tools
web/admin/tsconfig.json (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
strictTemplates: falsebroadly disables Vue template type-checking.This suppresses type errors across all templates rather than fixing the specific incompatibilities introduced by the Vue/TS/Arco upgrade, reducing the value of
vue-tsctype-checking going forward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/admin/tsconfig.json` around lines 18 - 20, Remove the vueCompilerOptions.strictTemplates override from the admin TypeScript configuration and resolve the specific Vue template type errors exposed by the Vue/TypeScript/Arco upgrade, preserving strict template checking for vue-tsc.web/admin/eslint.config.mjs (1)
54-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
no-debuggergated on build-timeNODE_ENV.The rule severity is fixed when ESLint's config loads based on
process.env.NODE_ENV, not per invocation intent. If CI doesn't explicitly setNODE_ENV=productionbefore linting,debuggerstatements won't be flagged even in a "production-bound" lint run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/admin/eslint.config.mjs` at line 54, Update the no-debugger configuration in the ESLint config so it is not gated by build-time process.env.NODE_ENV. Configure the rule to consistently flag debugger statements during linting, including production-bound CI runs regardless of the environment variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue`:
- Around line 86-90: Update the numeric value conversion used by
TopicAggregatorEditor’s model-value binding to return undefined when step.value
is an empty string before applying Number conversion, following the existing
craft_atom.vue helper pattern; preserve normal numeric conversion for non-empty
values.
---
Nitpick comments:
In `@web/admin/eslint.config.mjs`:
- Line 54: Update the no-debugger configuration in the ESLint config so it is
not gated by build-time process.env.NODE_ENV. Configure the rule to consistently
flag debugger statements during linting, including production-bound CI runs
regardless of the environment variable.
In `@web/admin/src/assets/style/tailwind.css`:
- Around line 1-2: Update the Stylelint configuration for
web/admin/src/assets/style/tailwind.css to allow Tailwind v4’s required `@config`
before `@import` ordering and recognize the `@config` at-rule. Scope the override to
this file where possible, disabling or ignoring
no-invalid-position-at-import-rule and scss/at-rule-no-unknown without changing
the stylesheet ordering.
In `@web/admin/tsconfig.json`:
- Around line 18-20: Remove the vueCompilerOptions.strictTemplates override from
the admin TypeScript configuration and resolve the specific Vue template type
errors exposed by the Vue/TypeScript/Arco upgrade, preserving strict template
checking for vue-tsc.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1240cb62-0714-456a-8497-6ee16908c3e1
⛔ Files ignored due to path filters (1)
web/admin/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
web/admin/.eslintignoreweb/admin/.eslintrc.jsweb/admin/babel.config.jsweb/admin/components.d.tsweb/admin/config/vite.config.dev.tsweb/admin/config/vite.config.prod.tsweb/admin/eslint.config.mjsweb/admin/package.jsonweb/admin/postcss.config.jsweb/admin/src/api/interceptor.tsweb/admin/src/assets/style/tailwind.cssweb/admin/src/env.d.tsweb/admin/src/utils/validator.tsweb/admin/src/views/dashboard/craft_atom/craft_atom.vueweb/admin/src/views/dashboard/craft_flow/CraftSelector.vueweb/admin/src/views/dashboard/craft_flow/components/CraftPickerModal.vueweb/admin/src/views/dashboard/curl_to_rss/curl_to_rss.vueweb/admin/src/views/dashboard/embedding_filter_debug/index.vueweb/admin/src/views/dashboard/feed_viewer/feed_viewer.vueweb/admin/src/views/dashboard/html_to_rss/html_to_rss.vueweb/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vueweb/admin/src/views/dashboard/topic_feed/components/TopicInputSourcesEditor.vueweb/admin/src/views/dashboard/topic_feed/detail.vueweb/admin/src/views/dashboard/web_monitor/web_monitor.vueweb/admin/tsconfig.jsonweb/admin/vitest.config.ts
💤 Files with no reviewable changes (5)
- web/admin/.eslintignore
- web/admin/src/utils/validator.ts
- web/admin/babel.config.js
- web/admin/.eslintrc.js
- web/admin/config/vite.config.dev.ts
| :model-value="toNumberValue(step.value)" | ||
| :min="1" | ||
| mode="button" | ||
| style="flex: 1; min-width: 150px" | ||
| @update:model-value="(value) => updateStepValue(idx, value)" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'TopicAggregatorEditor\.vue|craft_atom\.vue' . || true
echo "== inspect TopicAggregatorEditor relevant lines =="
if [ -f web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue ]; then
nl -ba web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue | sed -n '1,230p'
fi
echo "== search toNumberValue definitions/usages =="
rg -n "toNumberValue|Number\\(.*\\)|toNumber|value === ''|value == ''|''\\s*\\?|toNumber" web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue web/admin/src/views -S || true
echo "== inspect craft_atom relevant lines if found =="
while IFS= read -r f; do
echo "--- $f ---"
wc -l "$f"
sed -n '1,280p' "$f" | nl -ba
done < <(fd 'craft_atom\.vue' .)Repository: Colin-XKL/FeedCraft
Length of output: 438
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TopicAggregatorEditor.vue =="
awk '{printf "%6d\t%s\n", NR, $0}' web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue | sed -n '1,230p'
echo "== craft_atom.vue relevant searches =="
for pat in "toNumberValue|toNumber|Number\\("; do
echo "--- pattern: $pat ---"
grep -nE "$pat|value === ''|value == ''" web/admin/src/views/dashboard/craft_atom/craft_atom.vue || true
done
echo "== read-only behavioral probe for Number conversion and equality semantics =="
node - <<'JS'
function current(v) {
const r = Number(v);
return Number.isNaN(r) ? undefined : r;
}
for (const v of ["", "0", "1", "abc"]) {
console.log(JSON.stringify({value: v, current, modelMinOne: Number.isNaN(Number(v)) ? undefined : Math.max(1, Number(v)), rawNumberV: Number(v)}));
}
JSRepository: Colin-XKL/FeedCraft
Length of output: 8556
Preserve empty numeric values as undefined.
Number('') returns 0, so a numeric step with value === '' is rendered as 0 despite min="1". Handle value === '' before numeric conversion, matching the existing craft_atom.vue helper pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/admin/src/views/dashboard/topic_feed/components/TopicAggregatorEditor.vue`
around lines 86 - 90, Update the numeric value conversion used by
TopicAggregatorEditor’s model-value binding to return undefined when step.value
is an empty string before applying Number conversion, following the existing
craft_atom.vue helper pattern; preserve normal numeric conversion for non-empty
values.
Co-authored-by: Colin <Colin_XKL@outlook.com>
Co-authored-by: Colin <Colin_XKL@outlook.com>
Summary
px-16/py-8) to matchmain.Validation
pnpm outdated --format tablepnpm run lintpnpm run type:checkpnpm run buildpnpm run testtask frontend-buildmainfor AtomCraft and HTML to RSS.Manual walkthrough
main_upgrade_padding_comparison.mp4
To show artifacts inline, enable in settings.
Summary by Sourcery
Upgrade the admin frontend stack to current Vue, Vite, Tailwind, ESLint, and related tooling versions while applying necessary compatibility updates across the codebase.
Enhancements:
Build:
Tests:
Chores: