Update js-yaml to 5.2.1 - #28
Conversation
Dependency upgrade from 4.3.0 to 5.2.1. Split from Renovate PR #26.
EricWittmann
left a comment
There was a problem hiding this comment.
Code Review: Update js-yaml to 5.2.1
This PR upgrades js-yaml from 4.3.0 to 5.2.1 — a major version bump — along with the corresponding lockfile changes. The change was split from Renovate PR #26 for easier review.
Findings
⚠️ Warning
@types/js-yaml may need updating or removal (package.json)
The devDependencies still include @types/js-yaml at version 4.0.9, which provides type definitions for js-yaml 4.x. With js-yaml 5.x, one of the following is likely true:
- js-yaml 5.x ships its own built-in TypeScript types, making
@types/js-yamlredundant and potentially conflicting. In this case,@types/js-yamlshould be removed fromdevDependencies. - js-yaml 5.x does not ship types, in which case
@types/js-yamlshould be updated to a version that matches the 5.x API (if one exists).
The project has "skipLibCheck": true in tsconfig.json, which may mask type conflicts at build time, but it's still worth aligning the types to avoid IDE issues and incorrect type hints.
Recommendation: Verify whether js-yaml 5.x includes its own .d.ts files. If yes, remove @types/js-yaml. If no, check for a compatible @types/js-yaml version for 5.x.
💡 Suggestion
Verify API compatibility for dump() options (src/util/format.ts)
The codebase uses yaml.load() and yaml.dump() with options { lineWidth: -1, noRefs: true, sortKeys: false }. While these are core API functions that are likely stable across major versions, it's worth confirming that:
yaml.load()still accepts a single string argument and returns the parsed object (line 31)yaml.dump()still supports thelineWidth,noRefs, andsortKeysoptions (line 45)
Given that the PR description includes a test plan checklist ("Build passes", "Tests pass"), verifying those boxes are checked before merging would confirm there are no runtime regressions.
Overall Assessment
✅ Looks good to merge with one minor action item. The actual code changes are limited to the version bump in package.json and package-lock.json, and the API surface used by this project (load and dump) is minimal and well-established. The one item to address before or shortly after merging is the @types/js-yaml alignment — it should either be removed (if js-yaml 5.x bundles types) or updated to a compatible version.
Summary
Context
This dependency upgrade was split from Renovate PR #26 for easier review and testing.
Test Plan