Keep old deps working for changesets cli - #2303
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesets tooling is updated: the config schema URL is bumped to 3.1.4, ChangesDependency and Tooling Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Pull request overview
This PR updates repo-level tooling dependencies (Changesets + Hono) and refreshes the pnpm patch used for @changesets/assemble-release-plan, aiming to keep the Changesets CLI working after the dependency/security-upgrade follow-up to #2302.
Changes:
- Bump Changesets toolchain (
@changesets/cli,@changesets/changelog-github, config schema) and update the corresponding pnpm patch target/version. - Adjust pnpm overrides related to
js-yamland refresh the lockfile accordingly. - Bump
honocatalog version (and propagate through the lockfile).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Updates the catalog hono version used across workspaces. |
| pnpm-lock.yaml | Regenerates lockfile for the updated catalogs, overrides, and Changesets versions/patching. |
| patches/@changesets__assemble-release-plan@6.0.10.patch | Updates the pnpm patch to apply against @changesets/assemble-release-plan@6.0.10. |
| package.json | Bumps Changesets devDependencies; updates pnpm override selector for js-yaml; updates patchedDependencies mapping. |
| .changeset/config.json | Updates the $schema URL to match the newer @changesets/config version. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
patches/@changesets__assemble-release-plan@6.0.10.patch:12
- This patch effectively makes
shouldBumpMajorreturnfalsein all cases (the remaining return expression is only reachable whendepType !== peerDependencies, where it immediately short-circuits tofalse). That may be intentional, but the retained comment/logic about peerDependencies becomes misleading and makes future patch rebases harder. Consider rewriting the patched section so the intent is explicit (e.g., adjust/remove the now-irrelevant peerDependency major-bump logic rather than relying on a short-circuit + dead condition).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@greptile review |
Greptile SummaryThis PR upgrades the
Confidence Score: 5/5Safe to merge — all changes are dev-tooling dependency upgrades with no production code paths affected. The js-yaml patch is a correct and semantically equivalent alias (safeLoad→load, both are safe-by-default in 4.x). The assemble-release-plan patch is correctly rebased onto the new bundle. The @changesets and hono version bumps are routine. No logic changes to application code are introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["@changesets/cli@2.31.0"] --> B["@changesets/read@0.6.7"]
A --> C["@changesets/assemble-release-plan@6.0.10\n(patched: early-return for peerDeps)"]
B --> D["@changesets/parse@0.4.3"]
D --> E["js-yaml@4.2.0\n(patched)"]
C --> F["@manypkg/get-packages@1.1.3"]
F --> G["read-yaml-file@1.1.0"]
G --> E
E -->|"safeLoad → load alias"| H["✅ No runtime error"]
style E fill:#f0ad4e,color:#000
style H fill:#5cb85c,color:#fff
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["@changesets/cli@2.31.0"] --> B["@changesets/read@0.6.7"]
A --> C["@changesets/assemble-release-plan@6.0.10\n(patched: early-return for peerDeps)"]
B --> D["@changesets/parse@0.4.3"]
D --> E["js-yaml@4.2.0\n(patched)"]
C --> F["@manypkg/get-packages@1.1.3"]
F --> G["read-yaml-file@1.1.0"]
G --> E
E -->|"safeLoad → load alias"| H["✅ No runtime error"]
style E fill:#f0ad4e,color:#000
style H fill:#5cb85c,color:#fff
Reviews (3): Last reviewed commit: "Apply patch for `js-yaml` package" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| -module.exports.safeLoad = renamed('safeLoad', 'load') | ||
| -module.exports.safeLoadAll = renamed('safeLoadAll', 'loadAll') | ||
| -module.exports.safeDump = renamed('safeDump', 'dump') | ||
| +module.exports.safeLoad = module.exports.load | ||
| +module.exports.safeLoadAll = module.exports.loadAll | ||
| +module.exports.safeDump = module.exports.dump |
There was a problem hiding this comment.
@changesets/parse@0.4.3
In js-yaml 4.x, safeLoad, safeLoadAll, and safeDump intentionally throw via renamed() to force callers to migrate to load/loadAll/dump. These same packages (gray-matter@4.0.3, read-yaml-file@1.1.0) were already using js-yaml@4.2.0 without this patch before this PR, so they were either already migrated or never called the deprecated methods. The most likely trigger for needing this patch now is @changesets/parse@0.4.3, which means that package is calling a deprecated API. Silently re-aliasing those functions across the entire dependency tree hides the real issue and may cause other callers to silently use the deprecated path indefinitely. A more targeted fix would be filing/contributing upstream to @changesets/parse@0.4.3, or at minimum documenting which package triggered the need for this patch.
There was a problem hiding this comment.
The only package that was using js-yaml@3 was read-yaml-file@1.1.0.
read-yaml-file@1.1.0:
dependencies:
graceful-fs: 4.2.11
js-yaml: 3.14.2
pify: 4.0.1
strip-bom: 3.0.0
The upstream dependency (@changesets/parse@0.4.3) has a hard dependency on @manypkg/get-packages@1.1.3, which references read-yaml-file@1.1.0:.
The patch I've submitted is provides a direct alias to solve this issue:
> ensnode-monorepo@0.0.1 changeset:next /home/runner/_work/ensnode/ensnode
> changeset version --snapshot next && pnpm release:postversion
🦋 error Error: Function yaml.safeLoad is removed in js-yaml 4. Use yaml.load instead, which is now safe by default.
🦋 error at Object.safeLoad (/home/runner/_work/ensnode/ensnode/node_modules/.pnpm/js-yaml@4.2.0/node_modules/js-yaml/index.js:8:11)
🦋 error at parse (/home/runner/_work/ensnode/ensnode/node_modules/.pnpm/read-yaml-file@1.1.0/node_modules/read-yaml-file/index.js:8:28)
🦋 error at /home/runner/_work/ensnode/ensnode/node_modules/.pnpm/read-yaml-file@1.1.0/node_modules/read-yaml-file/index.js:10:71
There was a problem hiding this comment.
You're right, and the stack trace makes it clear — read-yaml-file@1.1.0 is calling yaml.safeLoad directly, and pnpm is resolving that to the hoisted js-yaml@4.2.0 which removed those functions. My original comment was wrong about the culprit being @changesets/parse.
The patch is a pragmatic and correct fix here: read-yaml-file is essentially abandoned, the upstream chain (@changesets/parse → @manypkg/get-packages → read-yaml-file) makes an upstream fix impractical, and the alias is semantically equivalent since load in js-yaml 4.x is already safe by default (it's what safeLoad was in 3.x).
No issue with the patch.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
patches/@changesets__assemble-release-plan@6.0.10.patch:12
- This patch makes
shouldBumpMajoreffectively always returnfalse: whendepTypeispeerDependenciesit returns early, and for all otherdepTypevalues the remainingreturn depType === "peerDependencies" && ...expression is necessarilyfalse. If the intent is only to change peer-dependency behavior (or to preserve upstream semantics), this condition looks inverted; if the intent is to disable major-bump inference entirely, the patch should be made explicit (and the now-dead upstream comment/return line should be adjusted) so future updates don’t misread the behavior.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Dang. Changesets is such a pain. Thanks for wrestling with this 👍
It's a follow up PR for #2302 which updates
@changesets/*dependencies to most recent version available. The PR also creates a patch for thejs-yamlpackage in order to:a) address security issues
b) resolve breaking changes introduced in the nest version of the package