search

catamphetamine / libphonenumber-js

A simpler (and smaller) rewrite of Google Android's libphonenumber library in javascript
https://catamphetamine.gitlab.io/libphonenumber-js/
MIT License
2.77k stars 218 forks source link

Recent version introduced hidden breaking change #384

Closed kamilchlebek closed 4 years ago

kamilchlebek commented 4 years ago

Observed result

AsYouType.input(text) breaks when given text is null

Expected result

Previously AsYouType.input(text) was working when given null

Demo

(open console too see output)

1.7.26 - working - https://stackblitz.com/edit/typescript-9hvxy1?file=index.ts 1.7.46 - failing - https://stackblitz.com/edit/typescript-erd9hr?file=index.ts

Here's the commit that introduced it: https://github.com/catamphetamine/libphonenumber-js/commit/4547faf8efb4c534fcb9073c2ea1892c960bffee#diff-0adfb6ac49dce262c372f3929c845c3eR1125-R1130

catamphetamine commented 4 years ago

Yes, there have been reports about that: https://github.com/catamphetamine/libphonenumber-js/issues/383 While this is indeed a breaking change, I think no one should have been passing null there in the first place. So I don't consider this an issue, even if it breaks someone's code. I'll reopen the original issue open though, so that others could see.

kamilchlebek commented 4 years ago

I agree with you. I've checked the CHANGELOG/releases in the first place, but there was no information about it. Adding it there will probably reduce number of issue reports.

catamphetamine commented 4 years ago

Adding it there will probably reduce number of issue reports.

That's a valid point. Added a note in the changelog.

The relevant change seems to be this big refactoring of AsYouType.js: https://github.com/catamphetamine/libphonenumber-js/commit/b66012dc70be42e3ecb3ae8863459be945a1034a#diff-0adfb6ac49dce262c372f3929c845c3e The version with the change seems to be 1.7.32.