chore(deps): bump js-yaml from 4.2.0 to 5.1.0#252
Conversation
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated code review by Claude Code review agent
Diff is manifest-only (package.json + package-lock.json); js-yaml is bumped 4.2.0 → 5.0.0 as a direct build dependency, no source/docs code changes. This is a major bump with real breaking changes (rewritten in TS, removed safeLoad/safeLoadAll/safeDump and DEFAULT_SCHEMA, Type API replaced by tags API, load() now throws on empty input) and it drops Node < 18.
Verdict: low-risk to merge provided this repo's CI docs-build job (and its Node version, must be ≥ 18) is green — that build is the real gate. js-yaml is used only by the Antora toolchain here, not by authored content, so a passing build is sufficient confidence. If CI is red, hold and check for a safeLoad/Type-API or Node-version regression.
|
@dependabot rebase |
e368c64 to
ee36689
Compare
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated review by Claude Code review agent.
Major version bump (js-yaml 4.2.0 → 5.1.0) — not a routine dependabot patch.
js-yaml is used directly in this repo's build toolchain, in ext-antora/load-global-site-attributes.js:
const yaml = require('js-yaml')
...
var d = yaml.load(data) // parses the global attributes file (local path or URL)js-yaml 5.x is a full rewrite (new API, new YAML implementation; 5.0.0 was released 2026-06-20, 5.1.0 today) with breaking changes that touch this exact call site. Per the v4→v5 migration guide:
- Empty input now throws.
load('')previously returnedundefined; in 5.x it throws. The attributes file here can come from a URL or local file — if it ever resolves empty,convert_yamlnow rejects and the extension callsthis.stop(), aborting the whole Antora build instead of silently continuing. Behavior change. - Default schema changed to YAML 1.2
CORE_SCHEMA, dropping\!\!merge(<<) and the YAML 1.1 types (\!\!timestamp,\!\!binary,\!\!set, and 1.1 int/float/bool syntax). If the global attributes YAML uses merge keys/anchors or 1.1-style scalars, parsing now changes or fails. Restoring requires passing an explicitschema:option that this code does not. - Flat exports. The migration guide documents
const { load } = require('js-yaml')but the code uses namespacedyaml.load; given "exports are now flat" and thebinentry moving to.mjs(ESM), the CommonJSyaml.loadproperty access should be re-verified against 5.x.
CI does not de-risk this: the green checks are Build documentation (10s) and lint. The load-global-site-attributes extension only runs when attributefile is configured in the playbook (site.yml), so a passing build does not prove the new parser handles the real attributes file.
Required action before merge: confirm (a) yaml.load still resolves under js-yaml 5.x CommonJS, and (b) the actual global attributes file parses identically (no << merge keys / YAML 1.1 types relied upon), or pin to ^4.x until the extension is migrated. A major bump of a directly-consumed YAML parser is not a zero-reservation change.
Checks: all green (Build documentation, lint — but neither exercises the js-yaml call path). Mergeable: CLEAN. Changelog: n/a (repo has no changelog).
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated review by Claude Code review agent.
Major bump (js-yaml 4.2.0 → 5.1.0), directly consumed in the build toolchain — but the breaking changes do not affect this repo's actual usage, and CI genuinely exercises the path.
This refines an earlier review that (a) flagged the breaking changes as a real risk and (b) claimed "CI does not exercise the js-yaml path." On closer inspection the second claim is incorrect, and the first does not apply here:
js-yaml is used directly, in ext-antora/load-global-site-attributes.js:
const yaml = require('js-yaml')
...
var d = yaml.load(data) // parses the global attributes fileCI does exercise this path. npm run antora (the CI "Build documentation" job) runs against site.yml, which has the extension enabled: true with attributefile: https://raw.githubusercontent.com/owncloud/docs/refs/heads/master/global-attributes.yml. So the green build actually invokes yaml.load on the real attributes file — it is not a dormant code path. CI runs Node 22, satisfying js-yaml 5.x's Node ≥ 18 requirement.
The documented v5 breaking changes do not hit this call site / content:
- Empty input now throws. The attributes URL currently resolves to a 72-line non-empty file, so
load()returns normally. (Residual edge: if that remote file ever became empty/unreachable,convert_yamlwould reject andthis.stop()aborts the build — but that already fails loudly today via the existing try/catch, it's not a silent regression.) - Default schema → CORE_SCHEMA (drops
\!\!merge/YAML-1.1 types). The attributes file uses no<<merge keys, no anchors/aliases, no\!\!tags, and onlytrue/falsebooleans (noyes/no/on/off/octal). Nothing it relies on is removed by CORE_SCHEMA. - Flat ESM exports /
bin→.mjs. Only thejs-yamlCLI bin moved to.mjs; the CommonJS namespacedyaml.load(...)used here still resolves under 5.x. The repo does not use the CLI.
Verdict: low-risk, approve-with-nits. A green build here is meaningful confidence because CI parses the actual attributes file with the new parser. The one nit: this depends on a remote attributes file that can change independently of this repo; should that file later adopt merge keys / YAML 1.1 types, the parse would change under CORE_SCHEMA without anything in this repo flagging it. Not blocking for this bump.
Checks: all green — Build documentation (exercises yaml.load), lint. Mergeable: CLEAN. Changelog: n/a (repo has no changelog).
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.2.0 to 5.1.0. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.2.0...5.1.0) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
ee36689 to
876432b
Compare
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated review by Claude Code review agent.
Dependabot: js-yaml 4.2.0 → 5.1.0 (MAJOR bump) — LOW RISK, approve-with-nits.
Diff: clean +6/-6 — package.json (^4.1.1→^5.1.0) plus the js-yaml lockfile node (4.2.0→5.1.0, bin changed bin/js-yaml.js→bin/js-yaml.mjs). No conflict markers; lockfile is well-formed.
Usage check: js-yaml is used at one site — ext-antora/load-global-site-attributes.js: const yaml = require('js-yaml') + yaml.load(data) (line 104). CommonJS require + yaml.load still work in 5.x.
5.x breaking changes vs. this call site (parsed file: owncloud/docs/master/global-attributes.yml, fetched remotely):
- Empty input now throws — file is non-empty (≈49 content lines); and an empty fetch would be caught by the extension's existing reject/error path.
- Default schema → CORE_SCHEMA (drops
\!\!merge, YAML-1.1 implicit types) — file uses onlytrue/falsebooleans, no merge keys, no anchors/aliases, no\!\!tags, no sexagesimals/yes-no-on-off. The one date-like value is an explicitly quoted string. - Flat ESM exports — not used (CommonJS named API
yaml.loadunaffected).
CI: Build documentation (which actually runs yaml.load) and lint both pass.
CHANGELOG: n/a (repo has no changelog).
Nit (pre-existing, not introduced here): the parsed attributes file is fetched from a remote master branch at build time, so this build depends on an external file that can change/empty independently of this repo. Not a blocker for this bump.
Bumps js-yaml from 4.2.0 to 5.1.0.
Changelog
Sourced from js-yaml's changelog.
... (truncated)
Commits
f1e45cd5.1.0 released53b22beFix constructor coveragea1eaa2bFix quote style options and restore forceQuotes0532e7dAdd finalizers for immutable collection tags9f00b91tests: drop the rest of issues tests, move a small fraction of useful checks ...6be5d46tests: drop not actual or duplicating issue tests (covered in other places)a7c9766Fix !!pairs coverage75148bc5.0.0 released704b25dQuote document markers followed by whitespace42dea28Support complex !!pairs keys with realMapTag