Release: Ensure newline at the end of package.json
#32505
Conversation
WalkthroughAdds a trailing newline to JSON writes in scripts/release/version.ts within bumpCodeVersion, bumpDeferred, and applyDeferredVersionBump. JSON content unchanged; only file termination gains a newline. No changes to control flow, logging, or exported interfaces. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
ndelangen
changed the title
package.json
ndelangen
added
build
ndelangen
deleted the
norbert/add-end-of-file-newline-to-packagejson
branch
|
View your CI Pipeline Execution ↗ for commit 98c9bc7
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
scripts/release/version.ts (4)
178-184: Inconsistent EOF behavior for other package.json files.This path still writes JSON without a trailing newline, so most package.json files won’t get the intended LF. Align with the change above.
Apply this minimal fix:
- await writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2)); + await writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2) + '\n');
96-99: Type bug:ExactOptions.exactshould be a version string, notsemver.ReleaseType.
ReleaseTypeis 'major'|'minor'|… and doesn’t represent exact versions like "7.2.0". This can mask errors and breaks type safety where a string version is expected.Apply:
-type ExactOptions = BaseOptions & { - exact: semver.ReleaseType; +type ExactOptions = BaseOptions & { + exact: string; deferred?: boolean; };
80-86: Runtime crash risk in validation when--exactis used with--pre-id.
schema.releaseTypecan be undefined, so.startsWith('pre')can throw.Use a safe check and keep the validation intent:
-if (schema.preId && !schema.releaseType.startsWith('pre')) { +if (schema.preId && (!schema.releaseType || !schema.releaseType.startsWith('pre'))) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'Using prerelease identifier requires one of release types: premajor, preminor, prepatch, prerelease', }); }
153-186: Fix: ensure bumped package.json files end with LFbumpAllPackageJsons writes JSON via
JSON.stringify(...)and callswriteFile(...)without adding a trailing newline; verification shows 47 package.json files under code/ missing a trailing LF. Change the write to append\n, e.g.writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2) + '\n').
Location: scripts/release/version.ts (lines 153–186).
🧹 Nitpick comments (2)
scripts/release/version.ts (2)
130-131: EOF newline addition looks good.Appends a single LF to code/package.json writes. Consider centralizing this behavior to avoid future drift (see comments below).
124-133: Avoid repetition: introduce a tiny helper for JSON writes with EOF LF.Reduces duplication and guarantees consistent newlines across all JSON outputs.
Example refactor:
import { readFile, writeFile } from 'node:fs/promises'; +// Helper to ensure JSON files end with a single LF +const writeJsonFile = (path: string, data: unknown) => + writeFile(path, JSON.stringify(data, null, 2) + '\n'); @@ - await writeFile(CODE_PACKAGE_JSON_PATH, JSON.stringify(codePkgJson, null, 2) + '\n'); + await writeJsonFile(CODE_PACKAGE_JSON_PATH, codePkgJson); @@ - await writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2) + '\n'); + await writeJsonFile(packageJsonPath, packageJson); @@ - await writeFile(CODE_PACKAGE_JSON_PATH, JSON.stringify(codePkgJson, null, 2) + '\n'); + await writeJsonFile(CODE_PACKAGE_JSON_PATH, codePkgJson); @@ - await writeFile(CODE_PACKAGE_JSON_PATH, JSON.stringify(codePkgJson, null, 2) + '\n'); + await writeJsonFile(CODE_PACKAGE_JSON_PATH, codePkgJson);Optional: if you ever need CRLF normalization, gate the terminator behind a constant, but LF is typically preferred and enforced by .gitattributes/formatters.
Also applies to: 188-208, 210-236, 153-186
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/release/version.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/release/version.ts (1)
code/frameworks/svelte-vite/src/plugins/generateDocgen.ts (1)
writeFile(387-389)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: docs
- GitHub Check: Danger JS
- GitHub Check: get-parameters
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
scripts/release/version.ts (2)
205-206: EOF newline addition for deferred write — LGTM.Matches the intent and mirrors the earlier change.
227-228: EOF newline addition on apply — LGTM.Consistent with other writes to code/package.json.
What I did
When we dropped
fs-extraand used nativewriteFile, it stopped having a newline at the end ofpackage.jsonThis causes issues during the release PRsChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Summary by CodeRabbit