refactor: serialize manifests through one shared helper - #405
Conversation
Three packages each had their own "write a manifest back preserving its
formatting" code, and two of them rewrote a CRLF manifest as LF:
- multi-semantic-release kept indent and trailing newline via
recognizeFormat, but serialized the body with LF;
- semantic-release-clean-package-json kept only the indent, through
writeJson({ detectIndent: true });
- semantic-release-pnpm kept indent, trailing newline and line endings.
Move the pnpm behaviour into shared/serialize-manifest.ts and use it in
all three. The folder is plain source that packem inlines into each
package at build time, so nothing new is published; the packages keep
declaring detect-indent and @visulima/fs themselves, and the root
manifest declares them too so the shared file type-checks in place.
recognizeFormat and the FileFormat/RecognizeFormatFunction types go
away with their only caller, as does the detect-newline dependency.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MqVjBDe8SkgMK7LpDpgjJ7
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Thank you for following the naming conventions! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/pnpm-prepare-single-write #405 +/- ##
=================================================================
+ Coverage 84.37% 86.04% +1.67%
=================================================================
Files 21 44 +23
Lines 448 1362 +914
Branches 108 356 +248
=================================================================
+ Hits 378 1172 +794
- Misses 69 178 +109
- Partials 1 12 +11
🚀 New features to boost your workflow:
|
Follow-up to #403 and #404 — based on #404's branch, so review it last; the base retargets automatically as those land.
Problem
Three packages each had their own "write a manifest back preserving its formatting" code, and two of them rewrote a CRLF manifest as LF — a whole-file diff in a release commit:
multi-semantic-release(recognizeFormat+writeFileSync)semantic-release-clean-package-json(writeJson({ detectIndent: true }))semantic-release-pnpm(after #404)Three implementations meant fixing one fixed none of the others.
Fix
shared/serialize-manifest.tsholds the pnpm behaviour as a pure function — manifest in, string out — and all three packages call it. No new npm package: the folder is plain source that packem inlines into eachdistat build time (verified:serializeManifestappears in the bundle of all three, withdetect-indentand@visulima/fs/eolstaying external).Being pure also makes it usable from
multi-semantic-release's synchronousupdateManifestDeps, which the asyncwriteJson-based approach could not be.Fallout:
recognizeFormat, its test, and theFileFormat/RecognizeFormatFunctiontypes go with their only caller, as does thedetect-newlinedependency.detect-indentis added tosemantic-release-clean-package-json(the other two already had it). Each package keeps declaring what the inlined code imports.detect-indentand@visulima/fsso the shared file resolves its own imports under pnpm's isolatednode_modules— without themtsc --noEmitfails onshared/serialize-manifest.ts.lint:prettier:rootnow also checksshared/*.ts. ESLint runs per package and does not reach the folder.Behaviour change
multi-semantic-releasepreviously appended the file's dominant newline whether or not the original ended with one; it now appends one only if the original did. That is the point of the helper — leave the file as it was found.Tests
packages/multi-semantic-release/__tests__/utils/serialize-manifest.test.ts— 8 cases covering indent, trailing newline, CRLF/LF and the no-original fallback. It replacesrecognize-format.test.tsand covers the helper on behalf of all three packages, since the folder has no test project of its own.semantic-release-clean-package-jsongains a CRLF round-trip test overpublish— it fails on the oldwriteJsonpath.update-deps.test.ts101/101,serialize-manifest8/8, clean-package-json 6/6, semantic-release-pnpm 12/12.tsc --noEmitclean in all three; all three build.multi-semantic-release.test.ts(23) andbin/cli.test.ts(2) fail in my sandbox — the fixture repos die ongit fetch --tagswith "couldn't find remote ref HEAD". I re-ran them against the base commit with the base dependency set: identical 23 failures, so they are environmental and not from this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01MqVjBDe8SkgMK7LpDpgjJ7