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

feat: add isNullish function #277

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add isNullish function #277

wants to merge 5 commits into from

Conversation

ilxqx
Copy link

@ilxqx ilxqx commented Oct 20, 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

Add isNullish function to check if the given value is null or undefined

Related issue, if any:

For any code change,

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

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1
A src/typed/isNullish.ts 53

Footnotes

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

@MarlonPassos-git
Copy link
Contributor

Thanks for the PR, @ilxqx. Just one thing: for functions that have type assertions, it's a good idea to include a type test to ensure that the types are working correctly. You can create the isNullish.test-d.ts file based on other test files, such as the isArray test.

@MarlonPassos-git
Copy link
Contributor

@ilxqx I find some lint errors in your changes, please run the pnpm lint and fix the errors.

@aleclarson The strange part is the CI catches the error, but they do not show the error in the check result.
image

I'm going to try to find the CI problem

@aleclarson
Copy link
Member

Hey @ilxqx, you should be able to run pnpm format to fix the formatting errors. Let me know if you encounter any difficulties. :)

@ilxqx
Copy link
Author

ilxqx commented Oct 21, 2024

@MarlonPassos-git I have added type test for isNullish and fixed the lint errors related to my committed file.

@ilxqx
Copy link
Author

ilxqx commented Oct 21, 2024

Hey @ilxqx, you should be able to run pnpm format to fix the formatting errors. Let me know if you encounter any difficulties. :)

I have run pnpm format and committed the changed files. It seems that everything is ok now.

@aleclarson aleclarson added the new feature This PR adds a new function or extends an existing one label Oct 26, 2024
@aeharding
Copy link
Contributor

I would suggest that the docs mention that value == null should be considered as an alternative, with proper lint rules to help enforce this pattern :)

@aleclarson
Copy link
Member

aleclarson commented Nov 6, 2024

@aeharding That's personally what I use, but it is somewhat prone to hard-to-debug typos, since people have a muscle memory for strict equals (===).

@aleclarson
Copy link
Member

aleclarson commented Nov 9, 2024

Hey! There's a new requirement for PRs that introduce new features. Without this requirement met, we won't be able to merge this. Note that this PR can still be included in a beta prerelease before this requirement is fulfilled.

⚠️ Note: You need to run git rebase main before this file will appear locally.

@radashi-bot
Copy link

Benchmark Results

Name Current
isNullish: with null 4,335,020.9 ops/sec ±0.07%
isNullish: with undefined 4,180,660.34 ops/sec ±0.07%
isNullish: with number 4,541,243.64 ops/sec ±0.06%
isNullish: with string 4,211,725.05 ops/sec ±0.07%
isNullish: with array 4,518,118.83 ops/sec ±0.06%

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 added prerelease Publish to NPM under the "beta" or "next" tag and removed prerelease Publish to NPM under the "beta" or "next" tag labels Nov 10, 2024
@aleclarson aleclarson added prerelease Publish to NPM under the "beta" or "next" tag and removed prerelease Publish to NPM under the "beta" or "next" tag labels Nov 11, 2024
radashi-bot pushed a commit that referenced this pull request Nov 11, 2024
radashi-bot pushed a commit that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This PR adds a new function or extends an existing one prerelease Publish to NPM under the "beta" or "next" tag
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants