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

semver fails when it comes to pre-releases #381

Closed Alan-Liang closed 4 years ago

Alan-Liang commented 4 years ago

Steps to reproduce

  1. Change the version in package.json from 1.7.44 to something like 1.7.44-build1
  2. npm run metadata:generate
  3. npm run test

Google's demo link

N/A

See also: https://github.com/substack/semver-compare/issues/1 .

catamphetamine commented 4 years ago

Hi. Ok, seems like a semver-compare bug, but what are the observed result and expected result of running the steps you've described. In other words, what's the exact issue you're having?

Alan-Liang commented 4 years ago

The exact issue is a test failure at https://github.com/catamphetamine/libphonenumber-js/blob/master/source/AsYouType.test.js#L623 , where formatter.input('7') is actually +8707 That is caused by https://github.com/catamphetamine/libphonenumber-js/blob/master/source/metadata.js treating metadata as V3.

catamphetamine commented 4 years ago

@Alan-Liang All tests pass both on my machine and on Travis CI.

Alan-Liang commented 4 years ago

A reproduction log: https://gist.github.com/Alan-Liang/b3576f18607dc4dba0eb2d616ebc7e05

catamphetamine commented 4 years ago

@Alan-Liang

cat package.json | grep version "version": "1.7.45-build1",

This library could potentially use alpha/beta releases but it doesn't. So this issue isn't really an issue for this library. If you want to maintain a fork with alpha/beta releases then you could fix that bug. The code is in the source folder (semver-compare isn't installed as a dependency, it's copy-pasted in the source code folder). So I won't be fixing this one because it's not an issue for me.

catamphetamine commented 4 years ago

@Alan-Liang A quick fix could be trimming everything after - in the version. You can submit a pull request with a fix (and with some unit tests).

catamphetamine commented 4 years ago

Actually, I could do this quick fix myself. Maybe in the evening.

On Wed, 4 Mar 2020 at 03:22, Alan Liang notifications@github.com wrote:

A reproduction log: https://gist.github.com/Alan-Liang/b3576f18607dc4dba0eb2d616ebc7e05

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/catamphetamine/libphonenumber-js/issues/381?email_source=notifications&email_token=AADUP33WQFLOXFJG5QNKZMDRFWNMZA5CNFSM4LAAZFJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENVWIHQ#issuecomment-594240542, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADUP322U2T56IWPLWZZXZLRFWNMZANCNFSM4LAAZFJA .

catamphetamine commented 4 years ago

Published libphonenumber-js@1.7.46. https://github.com/substack/semver-compare/issues/1#issuecomment-594765531