Skip to content

Improved type safety in edit recommendation modal#26124

Open
PedroMarianoAlmeida wants to merge 3 commits intoTryGhost:mainfrom
PedroMarianoAlmeida:remove-as-any-from-recomendation-modal
Open

Improved type safety in edit recommendation modal#26124
PedroMarianoAlmeida wants to merge 3 commits intoTryGhost:mainfrom
PedroMarianoAlmeida:remove-as-any-from-recomendation-modal

Conversation

@PedroMarianoAlmeida
Copy link

@PedroMarianoAlmeida PedroMarianoAlmeida commented Feb 3, 2026

This PR is a small code quality improvement without an associated
issue.

What does it do?

Removes an unsafe as any type cast from the edit recommendation modal
and replaces it with:

  • A type guard function (isRecommendation) with proper type predicate
  • Type-safe wrapper for the updateForm prop using
    React.ComponentProps
  • Runtime validation that provides clear error messages

Why?

I enjoy improving TypeScript type safety in open source projects as a
hobby. I've been doing similar work in other projects (like
react-bits and paypal-js#640), and
wanted to contribute the same improvements to Ghost.

Type casts with any bypass TypeScript's safety and can mask runtime
errors. This change makes the code fully type-checked while maintaining
identical behavior.

Impact

  • Users: No impact (internal change only)
  • Developers: Better type safety, clearer code, catches potential
    bugs at compile-time

Checklist

  • I've read and followed the [Contributor Guide](https://github.com
    /TryGhost/Ghost/blob/main/.github/CONTRIBUTING.md)
  • I've explained my change
  • Tests: All existing unit tests pass (126/126 ✓) and TypeScript
    compilation validates the types. No new tests needed as this is a type
    safety improvement with no behavioral changes.

Note

Low Risk
Type-safety refactor limited to the edit recommendation modal; behavior should be unchanged aside from a new explicit runtime error if an unexpected partial recommendation shape is returned.

Overview
Removes an unsafe as any cast when passing updateForm into RecommendationDescriptionForm in edit-recommendation-modal.tsx.

Adds an isRecommendation type guard and a typed updateFormForComponent wrapper (derived via React.ComponentProps) that enforces the edit modal always returns a full Recommendation shape, throwing a clear error if not.

Written by Cursor Bugbot for commit 9fa3449. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Refactor
    • Improved internal type handling for recommendation edits to ensure data integrity during form updates.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

A type guard and wrapper function were added to the recommendation edit modal to distinguish between complete and partial recommendation objects, enforcing that edit operations work with fully-formed Recommendation instances while supporting the more flexible EditOrAddRecommendation type.

Changes

Cohort / File(s) Summary
Type Safety Enhancement
apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx
Imported new EditOrAddRecommendation type; added isRecommendation() type guard to validate complete Recommendation objects; created updateFormForComponent() wrapper to enforce type constraints in edit mode and throw on failure.

Poem

🐰 A rabbit hops through types so true,
Distinguishing the old form from the new,
With guards in place and wrappers tight,
Our recommendations stay complete and right!
No partial edits shall slip through,
Full objects only—that's our view! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Improved type safety in edit recommendation modal' directly and accurately describes the main change: replacing unsafe type casts with type-safe alternatives using type guards and proper TypeScript patterns.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx`:
- Around line 14-16: The type guard isRecommendation should not only check
property presence but also validate runtime types: update the function
isRecommendation to verify typeof value.created_at === 'string', typeof value.id
=== 'string', and that value.updated_at is either null or a string (i.e.
value.updated_at === null || typeof value.updated_at === 'string'); keep the
existing property checks but replace them with these explicit typeof/null checks
to ensure robust discrimination between EditOrAddRecommendation and
Recommendation.

Comment on lines +14 to +16
const isRecommendation = (value: EditOrAddRecommendation | Recommendation): value is Recommendation => {
return 'created_at' in value && 'updated_at' in value && typeof value.id === 'string';
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "export type Recommendation" -A15 apps/admin-x-framework/src/api/recommendations.ts

Repository: TryGhost/Ghost

Length of output: 645


🏁 Script executed:

# Also check the EditOrAddRecommendation type
rg -n "export type EditOrAddRecommendation" -A15 apps/admin-x-framework/src/api/recommendations.ts

Repository: TryGhost/Ghost

Length of output: 732


🏁 Script executed:

# Check how isRecommendation is actually used in the file
rg -n "isRecommendation" -B2 -A2 apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx

Repository: TryGhost/Ghost

Length of output: 614


🏁 Script executed:

# Check for other type guards in the same file for comparison
rg -n "is.*=.*value is" apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx -B2 -A5

Repository: TryGhost/Ghost

Length of output: 439


🏁 Script executed:

# Look for similar type guard patterns across the admin-x-settings codebase
rg "value is.*=>.*typeof.*in" apps/admin-x-settings -B1 -A3 --max-count=5

Repository: TryGhost/Ghost

Length of output: 40


🏁 Script executed:

# Check if EditOrAddRecommendation can actually be passed to isRecommendation in practice
rg -n "EditOrAddRecommendation" apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx -B3 -A3

Repository: TryGhost/Ghost

Length of output: 883


Strengthen type validation in isRecommendation guard.

The type guard correctly distinguishes between EditOrAddRecommendation and Recommendation by checking for created_at and updated_at properties, but it only verifies their presence, not their types. Since created_at must be a string and updated_at can be string or null (per the Recommendation type), consider validating these types explicitly for runtime robustness.

Suggested enhancement
 const isRecommendation = (value: EditOrAddRecommendation | Recommendation): value is Recommendation => {
-    return 'created_at' in value && 'updated_at' in value && typeof value.id === 'string';
+    return typeof value === 'object'
+        && value !== null
+        && 'created_at' in value
+        && typeof value.created_at === 'string'
+        && 'updated_at' in value
+        && (typeof value.updated_at === 'string' || value.updated_at === null)
+        && typeof value.id === 'string';
 };
🤖 Prompt for AI Agents
In
`@apps/admin-x-settings/src/components/settings/growth/recommendations/edit-recommendation-modal.tsx`
around lines 14 - 16, The type guard isRecommendation should not only check
property presence but also validate runtime types: update the function
isRecommendation to verify typeof value.created_at === 'string', typeof value.id
=== 'string', and that value.updated_at is either null or a string (i.e.
value.updated_at === null || typeof value.updated_at === 'string'); keep the
existing property checks but replace them with these explicit typeof/null checks
to ensure robust discrimination between EditOrAddRecommendation and
Recommendation.

@PedroMarianoAlmeida
Copy link
Author

Should I do what the robot suggests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant