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 normalize for empty strings #315

Merged
merged 2 commits into from
Jun 26, 2023
Merged

Conversation

mikesposito
Copy link
Member

Currently the normalize function uses a logical NOT (!) to check if the input is nullish, but this causes to return undefined when an empty string is passed as input.

As a fix, this PR uses the isNullish function instead of the logical NOT.

Changes

  • FIXED: normalize function for empty strings

References

@mikesposito mikesposito requested a review from a team as a code owner June 22, 2023 12:47
@mikesposito mikesposito force-pushed the fix/normalize-for-empty-strings branch from f1cabce to 39fd8b7 Compare June 22, 2023 12:52
Copy link

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Can we add a test for this?

@legobeat
Copy link
Contributor

See also: #306

Copy link

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Looks good!

@mikesposito mikesposito merged commit 89035e2 into main Jun 26, 2023
@mikesposito mikesposito deleted the fix/normalize-for-empty-strings branch June 26, 2023 08:53
Gudahtt added a commit that referenced this pull request Jun 29, 2023
A test has been added to capture the behavior of the `normalize`
function when a zero is passed in. This behavior changed recently in
PR #315.
Gudahtt added a commit that referenced this pull request Jun 29, 2023
A test has been added to capture the behavior of the `normalize`
function when a zero is passed in. This behavior changed recently in
PR #315.
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.

3 participants