perf(info,upgrade): use confbox rather than pnpm-workspace-yaml - #1425
Conversation
commit: |
📝 WalkthroughWalkthroughThe catalog utility now uses Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/nuxt-cli/src/utils/catalog.ts (3)
397-403: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider quoting specifiers that YAML resolves as non-strings.
quoteYAMLScalarcovers indicator characters and whitespace, but not plain scalars that YAML resolves to another type. A specifier such as1.0or4is written unquoted and reads back as a number, soCatalogConfig.catalogsno longer matches its declaredRecord<string, string>type. Add a check for values that match a YAML number, boolean, or null 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 `@packages/nuxt-cli/src/utils/catalog.ts` around lines 397 - 403, Update quoteYAMLScalar to detect plain scalar values matching YAML number, boolean, or null forms and include them in needsQuotes, so specifiers such as “1.0” and “4” are serialized with JSON quoting while existing indicator and whitespace handling remains unchanged.
150-170: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRe-validate the edited document before writing.
The editor rewrites lines with hand-written string logic. A mis-detected block or an unusual specifier can produce a document that no longer parses. Add a parse of the joined result before
writeFileSync, and return'failed'if it does not parse. This keeps a corruptpnpm-workspace.yamlfrom reaching disk.♻️ Proposed guard
+ const output = lines.join('\n') + try { + parseYAML(output) + } + catch { + return 'failed' + } + try { - writeFileSync(filePath, lines.join('\n'), 'utf-8') + writeFileSync(filePath, output, 'utf-8') } catch { return 'failed' }🤖 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 `@packages/nuxt-cli/src/utils/catalog.ts` around lines 150 - 170, Before writeFileSync in the update flow, parse the edited document produced by lines.join('\n') using the existing catalog/workspace parser, and return 'failed' when parsing throws or otherwise fails. Only write the file after successful validation, preserving the existing unchanged and write-error outcomes.
218-227: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
splitTrailingCommenttreats#inside a quoted scalar as a comment.For a line such as
nuxt: "^4.0#1", the function returns the scalar"^4.0and the comment#1". A rewrite then emitsnuxt: ^4.2.0#1", which is malformed. The case is unlikely for package specifiers, so this is optional. The re-validation guard suggested onupdateCatalogEntrieswould turn this into a'failed'result instead of a corrupt file.🤖 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 `@packages/nuxt-cli/src/utils/catalog.ts` around lines 218 - 227, Update splitTrailingComment to recognize trailing # markers only when they occur outside quoted scalar content, preserving # characters inside single- or double-quoted values as part of the scalar. Ensure updateCatalogEntries re-validates the rewritten content and returns 'failed' rather than emitting malformed output when parsing or validation still fails.
🤖 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 `@packages/nuxt-cli/src/utils/catalog.ts`:
- Around line 371-380: The catalog insertion logic around findBlock must use the
existing catalogs child indentation instead of always using catalogs.indent + 2.
Return or derive the child indent from findBlock (using the first child entry
when present), fall back to catalogs.indent + 2 only for an empty block, and
apply that indent to both the new catalog key and entry; add coverage for a
four-space-indented catalogs block in the catalog utility tests.
- Around line 199-210: Update unescapeYAMLString to handle JSON.parse failures
from YAML-only escape sequences without throwing, and return a safe unescaped
value consistent with the documented failed-result flow. Ensure parseKeyLine
cannot propagate this exception through updateCatalogEntries, while preserving
existing single-quoted handling and valid double-quoted escape behavior.
---
Nitpick comments:
In `@packages/nuxt-cli/src/utils/catalog.ts`:
- Around line 397-403: Update quoteYAMLScalar to detect plain scalar values
matching YAML number, boolean, or null forms and include them in needsQuotes, so
specifiers such as “1.0” and “4” are serialized with JSON quoting while existing
indicator and whitespace handling remains unchanged.
- Around line 150-170: Before writeFileSync in the update flow, parse the edited
document produced by lines.join('\n') using the existing catalog/workspace
parser, and return 'failed' when parsing throws or otherwise fails. Only write
the file after successful validation, preserving the existing unchanged and
write-error outcomes.
- Around line 218-227: Update splitTrailingComment to recognize trailing #
markers only when they occur outside quoted scalar content, preserving #
characters inside single- or double-quoted values as part of the scalar. Ensure
updateCatalogEntries re-validates the rewritten content and returns 'failed'
rather than emitting malformed output when parsing or validation still fails.
🪄 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: cbdb887a-21a4-4f5b-8b12-e803c3467947
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/nuxt-cli/package.jsonpackages/nuxt-cli/src/utils/catalog.tspackages/nuxt-cli/test/unit/utils/catalog.spec.ts
💤 Files with no reviewable changes (1)
- packages/nuxt-cli/package.json
| const key = quote ? quoted! : plain!.trimEnd() | ||
| return { | ||
| indent: match[1]!.length, | ||
| key: quote ? unescapeYAMLString(key, quote) : key, | ||
| raw: quote ? `${quote}${quoted}${quote}` : key, | ||
| value: rest!.trimStart(), | ||
| } | ||
| } | ||
|
|
||
| function unescapeYAMLString(value: string, quote: string): string { | ||
| return quote === '\'' ? value.replaceAll('\'\'', '\'') : JSON.parse(`"${value}"`) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard unescapeYAMLString against a JSON.parse throw.
JSON.parse rejects YAML-only escapes such as \x41, \e, or \_. parseKeyLine runs on every line of the workspace file, including lines the caller does not target. A double-quoted key with such an escape makes JSON.parse throw. The throw escapes updateCatalogEntries, because the try block there only wraps the read and the initial parse. The documented contract is to return 'failed' instead.
🛡️ Proposed fix
function unescapeYAMLString(value: string, quote: string): string {
- return quote === '\'' ? value.replaceAll('\'\'', '\'') : JSON.parse(`"${value}"`)
+ if (quote === '\'') {
+ return value.replaceAll('\'\'', '\'')
+ }
+ try {
+ return JSON.parse(`"${value}"`)
+ }
+ catch {
+ return value
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const key = quote ? quoted! : plain!.trimEnd() | |
| return { | |
| indent: match[1]!.length, | |
| key: quote ? unescapeYAMLString(key, quote) : key, | |
| raw: quote ? `${quote}${quoted}${quote}` : key, | |
| value: rest!.trimStart(), | |
| } | |
| } | |
| function unescapeYAMLString(value: string, quote: string): string { | |
| return quote === '\'' ? value.replaceAll('\'\'', '\'') : JSON.parse(`"${value}"`) | |
| } | |
| const key = quote ? quoted! : plain!.trimEnd() | |
| return { | |
| indent: match[1]!.length, | |
| key: quote ? unescapeYAMLString(key, quote) : key, | |
| raw: quote ? `${quote}${quoted}${quote}` : key, | |
| value: rest!.trimStart(), | |
| } | |
| } | |
| function unescapeYAMLString(value: string, quote: string): string { | |
| if (quote === '\'') { | |
| return value.replaceAll('\'\'', '\'') | |
| } | |
| try { | |
| return JSON.parse(`"${value}"`) | |
| } | |
| catch { | |
| return value | |
| } | |
| } |
🤖 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 `@packages/nuxt-cli/src/utils/catalog.ts` around lines 199 - 210, Update
unescapeYAMLString to handle JSON.parse failures from YAML-only escape sequences
without throwing, and return a safe unescaped value consistent with the
documented failed-result flow. Ensure parseKeyLine cannot propagate this
exception through updateCatalogEntries, while preserving existing single-quoted
handling and valid double-quoted escape behavior.
| const catalogs = findBlock(lines, ['catalogs']) | ||
| if (catalogs === 'failed') { | ||
| return 'failed' | ||
| } | ||
| if (!catalogs) { | ||
| return appendLines(lines, ['catalogs:', ` ${catalog}:`, ` ${entry}`]) | ||
| } | ||
|
|
||
| lines.splice(catalogs.end, 0, `${' '.repeat(catalogs.indent + 2)}${catalog}:`, `${' '.repeat(catalogs.indent + 4)}${entry}`) | ||
| return 'updated' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Derive the child indent from the existing catalogs children.
Line 379 hardcodes catalogs.indent + 2 for the new catalog name. If the file already indents the children of catalogs by 4 spaces, the inserted key sits at indent 2 while its siblings sit at indent 4. YAML requires all keys of one mapping to share the same indentation, so the resulting file fails to parse.
Reproduce with:
catalogs:
dev:
typescript: ^5.9.0Return the child indent from findBlock (or scan the block for the first entry line) and use it here, with + 2 only as the fallback for an empty block. Add a 4-space test case in packages/nuxt-cli/test/unit/utils/catalog.spec.ts.
🤖 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 `@packages/nuxt-cli/src/utils/catalog.ts` around lines 371 - 380, The catalog
insertion logic around findBlock must use the existing catalogs child
indentation instead of always using catalogs.indent + 2. Return or derive the
child indent from findBlock (using the first child entry when present), fall
back to catalogs.indent + 2 only for an empty block, and apply that indent to
both the new catalog key and entry; add coverage for a four-space-indented
catalogs block in the catalog utility tests.
1 participant
🔗 Linked issue
📚 Description
pnpm-workspace-yaml+yamlwere 677 KB; confbox was already in the dependency tree, so this is almost free...