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

latest patch version (1.2.3) changes behavior if string is undefined #19

Closed
sballesteros opened this issue Mar 21, 2018 · 3 comments
Closed

Comments

@sballesteros
Copy link

sballesteros commented Mar 21, 2018

The change from re.test(string) to string.match(re) result in a different behavior if string is undefined (calling .test on undefined will throw a TypeError).
Maybe put a guard like if (typeof string == 'string') return false to avoid that or consider bumping the major version as 1.2.3 can be considered as a change introducing a breaking change.

davisjam added a commit to davisjam/is-url that referenced this issue Mar 24, 2018
Problem:
As reported in segmentio#19, PR segmentio#18 changed behavior on:
- undefined string
- non-strings

Solution:
Revert to pre-segmentio#18 behavior on non-strings: return false.

Test:
I added test cases to clarify this behavior.
It shouldn't happen again.
@davisjam
Copy link
Contributor

@sballesteros @cadente-nd Thank you for pointing this out.

This is a bug in #18. I was trying to keep all existing behavior the same, but there was no test case for this so I missed it.

#20 will fix this issue.

davisjam added a commit to davisjam/is-url that referenced this issue Mar 26, 2018
Problem:
As reported in segmentio#19, PR segmentio#18 changed behavior on:
- undefined string
- non-strings

Solution:
Revert to pre-segmentio#18 behavior on non-strings: return false.

Test:
I added test cases to clarify this behavior.
It shouldn't happen again.
@davisjam
Copy link
Contributor

@sballesteros If #20 solves your problem, can you close this issue?

#20 is available in is-url version 1.2.4 on npm.

@sballesteros
Copy link
Author

@davisjam thanks!

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

No branches or pull requests

2 participants