Skip to content

Conversation

@Yeom-JinHo
Copy link
Contributor

@Yeom-JinHo Yeom-JinHo commented Nov 7, 2025

Description

This PR updates the isSafeInteger utility to behave as a proper TypeScript type predicate.
Previously, it only returned a boolean value, which did not narrow the input type.
Now it returns value is number, aligning with the JSDoc description and enabling type narrowing in conditional branches.


Changes

  • Type Predicate Update (major fix):
    Updated the return type from booleanvalue is number.
    This allows TypeScript to narrow the argument type to number when the function returns true.

  • Parameter Type Refinement:
    Changed the parameter type from anyunknown for safer type inference.

  • Documentation Update:
    Modified MDX docs:

    • Updated Parameters and Returns table to reflect the new type predicate.
    • Added a TypeScript note section explaining type narrowing behavior.
  • Type Test Update:
    Added a dedicated type predicate test case verifying that the value is narrowed to number inside conditional blocks.


Motivation

The previous implementation’s JSDoc claimed that it could “serve as a type predicate,”
but in reality, it didn’t provide any type narrowing because the function signature returned a plain boolean.
This update ensures that the function now truly behaves as a type guard, matching both runtime behavior and TypeScript semantics.


Breaking Changes

  • No runtime behavior changes.
  • Type-level change:
    Projects relying on an explicit boolean return type may need to update dependent type definitions or mocks.

@Yeom-JinHo Yeom-JinHo requested a review from raon0211 as a code owner November 7, 2025 14:32
Copilot AI review requested due to automatic review settings November 7, 2025 14:32
@Yeom-JinHo Yeom-JinHo requested a review from dayongkr as a code owner November 7, 2025 14:32
@vercel
Copy link

vercel bot commented Nov 7, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
es-toolkit Ready Ready Preview Comment Nov 13, 2025 9:01am

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the isSafeInteger function to use TypeScript's type predicate feature, enabling better type narrowing. The function signature is updated from returning boolean to returning value is number, and the parameter type is improved from any to unknown.

Key Changes

  • Updated function signature to use TypeScript type predicate (value is number)
  • Changed parameter type from any to unknown for better type safety
  • Updated JSDoc comments and documentation across all language versions to reflect the type predicate behavior

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/compat/predicate/isSafeInteger.ts Updated function signature to use type predicate and improved parameter type from any to unknown
docs/reference/compat/predicate/isSafeInteger.md Added documentation explaining the type predicate behavior in English
docs/zh_hans/reference/compat/predicate/isSafeInteger.md Added documentation explaining the type predicate behavior in Chinese
docs/ko/reference/compat/predicate/isSafeInteger.md Added documentation explaining the type predicate behavior in Korean
docs/ja/reference/compat/predicate/isSafeInteger.md Added documentation explaining the type predicate behavior in Japanese

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@raon0211 raon0211 left a comment

Choose a reason for hiding this comment

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

Thanks!

@raon0211 raon0211 merged commit eb14e19 into toss:main Nov 13, 2025
1 of 7 checks passed
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.

2 participants