-
Notifications
You must be signed in to change notification settings - Fork 8.2k
chore: update deps #6814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
chore: update deps #6814
Conversation
|
WalkthroughUpdates the root packageManager to [email protected], bumps multiple dependency versions in pnpm-workspace.yaml, and applies a minor formatting change (trailing comma) in a TypeScript utility file. No logic, control flow, or exported API changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/effects/plugins/src/echarts/use-echarts.ts (1)
60-68
: Bug: theme argument is ignored when set to 'light' due to operator precedence
t || isDark.value ? 'dark' : null
evaluates as(t || isDark.value) ? 'dark' : null
, forcing 'dark' even whent === 'light'
. Use nullish coalescing or parentheses.Apply this fix:
- chartInstance = echarts.init(el, t || isDark.value ? 'dark' : null); + // Use provided theme if defined; otherwise use dark when preference is dark + chartInstance = echarts.init(el, t ?? (isDark.value ? 'dark' : null));pnpm-workspace.yaml (1)
16-194
: Fixcheck-circular
script failure
vsh check-circular
errors with ERR_MODULE_NOT_FOUND: cannot findjiti/lib/jiti.mjs
(seeinternal/node-utils
/scripts/vsh
). Ensure thejiti
dependency is installed at the expected version or update the import paths inscripts/vsh
andinternal/node-utils
.- Rerun full CI (pnpm run check, turbo run typecheck, build, test:unit, test:e2e) and confirm all steps pass.
🧹 Nitpick comments (2)
packages/effects/plugins/src/echarts/use-echarts.ts (1)
70-105
: Unbounded retries when element stays hiddenRecursive defers via
useTimeoutFn(... renderEcharts(...), 30)
have no cap, causing potential infinite retry/timer churn if the element never becomes visible.Consider a max-attempt counter or an AbortSignal. Example:
- const renderEcharts = ( - options: EChartsOption, - clear = true, - ): Promise<Nullable<echarts.ECharts>> => { + const renderEcharts = ( + options: EChartsOption, + clear = true, + _attempt = 0, + ): Promise<Nullable<echarts.ECharts>> => { cacheOptions = options; const currentOptions = { ...options, ...getOptions.value, }; return new Promise((resolve) => { - if (chartRef.value?.offsetHeight === 0) { + if (chartRef.value?.offsetHeight === 0) { + if (_attempt > 50) return resolve(null); useTimeoutFn(async () => { - resolve(await renderEcharts(currentOptions)); + resolve(await renderEcharts(currentOptions, clear, _attempt + 1)); }, 30); return; } nextTick(() => { const el = getChartEl(); if (isElHidden(el)) { + if (_attempt > 50) return resolve(null); useTimeoutFn(async () => { - resolve(await renderEcharts(currentOptions)); + resolve(await renderEcharts(currentOptions, clear, _attempt + 1)); }, 30); return; }package.json (1)
101-102
: Align engines.pnpm with packageManager to avoid toolchain skewYou updated
packageManager
to[email protected]
, butengines.pnpm
is>=9.12.0
. Recommend aligning to the same major (and ideally exact/min required) to reduce CI/dev mismatches.Proposed tweak:
"engines": { "node": ">=20.10.0", - "pnpm": ">=9.12.0" + "pnpm": ">=10.18.2" }, "packageManager": "[email protected]",Please confirm:
- Corepack is enabled and
pnpm 10.18.2
is prepared in CI.- A fresh install works:
pnpm -v
shows 10.18.2 andpnpm -w install
succeeds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json
(1 hunks)packages/effects/plugins/src/echarts/use-echarts.ts
(1 hunks)pnpm-workspace.yaml
(3 hunks)
⏰ 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). (2)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (1)
packages/effects/plugins/src/echarts/use-echarts.ts (1)
70-74
: Nit: trailing comma change is fineNo behavior change; signature remains compatible.
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit