fix(cli): update command should preserve existing package.json order #2810
Conversation
🦋 Changeset detectedLatest commit: 969145c The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR adds functionality to preserve JSON key ordering when writing files in the CLI. It introduces two new utility functions— Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![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
🧹 Nitpick comments (2)
packages/cli-v3/src/utilities/fileSystem.ts (2)
162-170: Consider adding a trailing newline for better POSIX compliance.The implementation is correct, but many tools and editors expect text files to end with a newline character. Consider appending
\nto the stringified content.🔎 Suggested enhancement
export async function writeJSONFile(path: string, json: any, pretty = false) { - await safeWriteFile(path, stringify(json, pretty ? { space: 2 } : undefined) ?? ""); + const content = stringify(json, pretty ? { space: 2 } : undefined) ?? ""; + await safeWriteFile(path, content + (content ? "\n" : "")); }
172-180: Consider adding a trailing newline for consistency.The implementation correctly preserves key order using native
JSON.stringify. However, the same trailing newline consideration fromwriteJSONFileapplies here for better POSIX compliance.🔎 Suggested enhancement
export async function writeJSONFilePreserveOrder(path: string, json: any, pretty = false) { - await safeWriteFile(path, JSON.stringify(json, undefined, pretty ? 2 : undefined)); + const content = JSON.stringify(json, undefined, pretty ? 2 : undefined); + await safeWriteFile(path, content + "\n"); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/cool-elephants-carry.mdpackages/cli-v3/src/commands/update.tspackages/cli-v3/src/utilities/fileSystem.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/commands/update.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/commands/update.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/commands/update.ts
🧠 Learnings (2)
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: Use pnpm for package management in this monorepo
Applied to files:
packages/cli-v3/src/commands/update.ts
📚 Learning: 2025-11-27T16:26:44.496Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/executing-commands.mdc:0-0
Timestamp: 2025-11-27T16:26:44.496Z
Learning: Execute most monorepo commands using `pnpm run` from the root directory, with `--filter` flag for specific packages (e.g., `pnpm run dev --filter webapp`)
Applied to files:
packages/cli-v3/src/commands/update.ts
🧬 Code graph analysis (2)
packages/cli-v3/src/utilities/fileSystem.ts (1)
packages/core/src/v3/apps/http.ts (1)
json(65-75)
packages/cli-v3/src/commands/update.ts (1)
packages/cli-v3/src/utilities/fileSystem.ts (1)
writeJSONFilePreserveOrder(178-180)
⏰ 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). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/cool-elephants-carry.md (1)
1-5: LGTM!The changeset properly documents this patch-level fix with a clear description that matches the PR objectives.
packages/cli-v3/src/commands/update.ts (2)
4-4: LGTM! Import changes are correct.The removal of the unused
basenameimport and the switch towriteJSONFilePreserveOrderproperly align with the PR objectives to preserve package.json key ordering.Also applies to: 9-9
230-230: Excellent fix! All package.json operations now preserve key order.The three usages correctly apply
writeJSONFilePreserveOrderto:
- Backup the original package.json
- Write the updated package.json
- Revert to the original package.json
This properly addresses the regression while maintaining the user's preferred key ordering.
Also applies to: 244-244, 247-247
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
This fixes a regression introduced in #2778 - stable sort is required for deterministic builds, but we can safely preserve order for the user package.json during package updates