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): align isPromise return type with its logic #175

Merged
merged 4 commits into from
Aug 16, 2024

Conversation

aleclarson
Copy link
Member

@aleclarson aleclarson commented Aug 16, 2024

Tip

The owner of this PR can publish a preview release by commenting /publish in this PR. Afterwards, anyone can try it out by running pnpm add radashi@pr<PR_NUMBER>.

Summary

Change the return type from Promise<any> to PromiseLike<unknown>. This is more representative of the actual check being done.

Note: This type guard assumes a "then" method is only ever used for promise-like behavior. Please avoid using this function if you can't ensure such a thing.

Related issue, if any:

Inspired by https://github.com/orgs/radashi-org/discussions/73. This PR does not resolve that issue. I still think we need to rename this function to isPromiseLike, but that is a breaking change, so it needs its own PR.

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1 Difference (%)
M src/async/tryit.ts 240 -7 (-3%)
M src/typed/isPromise.ts 101 +0 (+0%)

Footnotes

  1. Function size includes the import dependencies of the function.

@radashi-bot
Copy link

radashi-bot commented Aug 16, 2024

Benchmark Results

Name Current Baseline Change
isPromise ▶︎ with Promise 1,602,641.21 ops/sec ±3.44% 2,187,991.31 ops/sec ±0.1% 🔗 🐢 -26.75%
isPromise ▶︎ with Promise-like 3,861,845.61 ops/sec ±0.07% 4,081,600.69 ops/sec ±0.06% 🔗 🐢 -5.38%
isPromise ▶︎ with non-Promise 3,597,606.25 ops/sec ±0.07% 4,019,259.49 ops/sec ±0.06% 🔗 🐢 -10.49%

Performance regressions of 30% or more should be investigated, unless they were anticipated. Smaller regressions may be due to normal variability, as we don't use dedicated CI infrastructure.

@aleclarson aleclarson merged commit d6e0dff into main Aug 16, 2024
7 checks passed
@aleclarson aleclarson deleted the fix/isPromise-return-type branch August 16, 2024 01:30
Copy link

A new beta version 12.2.0-beta.6b76988 has been published to NPM. 🚀

To install:

pnpm add radashi@12.2.0-beta.6b76988

The radashi@beta tag also includes this PR.

See the changes

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