Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(types): improve draw signature for non-empty arrays #153

Merged
merged 11 commits into from
Nov 10, 2024

Conversation

crishoj
Copy link
Contributor

@crishoj crishoj commented Aug 6, 2024

Summary

This is a renewed attempt at narrowing the return type for draw(), so that emptiness of the array argument is taken into account.

  • For literal array arguments, return type can be determined as T or null.
  • For mutable array arguments, return type remains T|null.
  • For immutable array arguments,
    • return type remains T|null for empty arrays (not sure why)
    • return type is narrowed to T for non-empty arrays.
image

Disclosure: I am a Typescript novice and welcome your feedback.

If the approach is sound, similar overrides could be considered for

  • first
  • last

Related issue, if any:

For any code change,

  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

src/random/draw.ts Outdated Show resolved Hide resolved
@MarlonPassos-git
Copy link
Contributor

Good job @crishoj, now we don't have that false positive.

Copy link
Contributor

@MarlonPassos-git MarlonPassos-git left a comment

Choose a reason for hiding this comment

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

Deleting that line is all ok

@radashi-bot
Copy link

radashi-bot commented Aug 15, 2024

Benchmark Results

Name Current Baseline Change
draw ▶︎ with valid input 1,068,168.13 ops/sec ±0.33% 1,088,828.86 ops/sec ±0.58% 🔗 🐢 -1.9%

Generally speaking, any regression ≥20% should be investigated if it wasn't to be expected.

@aleclarson aleclarson added this to the v12.3.0 milestone Sep 21, 2024
@aleclarson aleclarson added the better types This PR mainly improves type definitions label Sep 21, 2024
@aleclarson aleclarson changed the title feat(draw): narrow return type for empty and non-empty arrays (take II) fix(types): improve draw signature for non-empty arrays Nov 9, 2024
@aleclarson aleclarson removed this from the v12.3.0 milestone Nov 10, 2024
@aleclarson aleclarson force-pushed the narrow-return-type-for-draw branch from 5903445 to 0835852 Compare November 10, 2024 14:24
@aleclarson
Copy link
Member

I've updated the signature to ensure inlined arrays are properly handled at the type level:

const val = draw([1, 2, 3])
//    ^? 1 | 2 | 3

@aleclarson aleclarson merged commit fee290a into radashi-org:main Nov 10, 2024
8 checks passed
@radashi-bot
Copy link

A stable release 12.2.2 has been published to NPM. 🚀

To install:

See the changes

@crishoj
Copy link
Contributor Author

crishoj commented Nov 10, 2024

Excellent, thanks for the improvement.

T[number] to get element type — TIL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better types This PR mainly improves type definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants