Invalid regular expression on starting the service after upgrading to 13.1.0

[mkumarb]

SyntaxError: Invalid regular expression: /^((((+|0{2})3876)|06)[0-6])((?<=4)\d{7}|(?<!4)\d{6})$/: Invalid group
Did not have this until yesterday (when I was on v13.0.0). When I was looking at npm, saw that a new version was upgraded just around 20 hours ago. Also the regular expression pointed out in the error was added in the new version:
Click here to view the changeset which includes the below regular expression

Error on starting service when the module is required:

/usr/app/node_modules/validator/lib/isMobilePhone.js:14
var phones = {
             ^
SyntaxError: Invalid regular expression: /^((((\+|0{2})3876)|06)[0-6])((?<=4)\d{7}|(?<!4)\d{6})$/: Invalid group
    at Object.<anonymous> (/usr/app/node_modules/validator/lib/isMobilePhone.js:14:14)
    at Module._compile (module.js:643:30)
    at Object.Module._extensions..js (module.js:654:10)
    at Module.load (module.js:556:32)
    at tryModuleLoad (module.js:499:12)
    at Function.Module._load (module.js:491:3)

Validator.js version: 13.1.0
Node.js version: v8.9.4
OS platform: macOS

Happened when running app in a docker container.

[egges]

Same thing happening here, but specifically on Safari (on Chrome there is no error), and not only in a Docker container, but also in local development.

[novemberborn]

(?<=4) and (?<!4) are lookbehind assertions. Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#Browser_compatibility they're supported in Node.js 8.10 and various browsers, but not Safari.

This package doesn't explicitly list the minimal JS requirements. It should work on all maintained Node.js versions (10 and above) but the browser compatibility is a concern.

Incidentally this breaks this module in React Native projects.

[profnandaa]

We definitely are pro backward compatibility, that was an oversight from our end; working on fixing that asap.

[profnandaa]

@MladenZeljic -- could you please have a look at this and do a hot-fix?

[itxtoledo]

same issue here, using iphone browsers and safari on macOS

[tux-tn]

@profnandaa I have already seen one or two PR using lookbehind assertions in the past (as an example #1244).
We can start running test on other platforms than nodejs to prevent those errors.

[MladenZeljic]

Hey @profnandaa thanks for notifying me about this and sorry about this oversight. I'll work on replacing these lookbehinds with something less "sugary". And I agree with @tux-tn

[wendt88]

same here for nativescript on iOS devices only

[profnandaa]

patched version was released yesterday, can you check? 13.1.1

[profnandaa]

@tux-tn -- you know the easiest way we can do this (browser tests) with our current TravisCI pipeline?

[profnandaa]

I noticed that this was also regressing on Node v6, so I have added v6 on our TravisCI pipeline for the time being to act as a proxy - #1357 ; but we need a browser solution if it's proximal.

[tux-tn]

@profnandaa i know that mocha can run on browsers, i will try to work on it this week end. I'm also planning to add some missing tests (Actual coverge decreased from 100% to 98.16%)

[profnandaa]

Thanks! ./na

On Fri, Jun 12, 2020 at 4:34 PM Sarhan Aissi ***@***.***> wrote: @profnandaa <https:/profnandaa> i know that mocha can run on browsers, i will try to work on it this week end. I'm also planning to add some missing tests (Actual coverge decreased from 100% to 98.16%) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1354 (comment)>, or unsubscribe <https:/notifications/unsubscribe-auth/AAB7ZEKL33G2SGIZKVBZZ5TRWIVG7ANCNFSM4N3CJ5FA> .

-- Sent from a tiny device while on the move.

[profnandaa]

Forgot to close this, I think we should be good now? Let me know if there's anything further that needs to be done other than fixing our CI/CD scope.