A bit weird looking regex

[JohnLindahlTech]

next-auth/src/lib/parse-url.js

Line 13 in 8115a7c

const protocol = url.match(/^http?:\/\//) ? 'http' : 'https'

The regex /^http?:\/\// will match http:// but also htt://.
Is this really intended, or should the ? be removed on this line? (It is ofcourse needed later in the https? replace regex.)

[balazsorban44]

It probably misses an s before that question mark.

update: sorry, I see the purpose now. Maybe this could be an easier and more standard way of getting the protocol

[JohnLindahlTech]

Yes that sounds like a good idea

[Zertz]

URL wasn't always available as a global in Node nor browsers so I would guess manually parsing the string is the best way to ensure the best compatibility without adding dependencies.

[JohnLindahlTech]

Ah ok, yeah sounds fair. but then it's just a matter of removing the ? in that specific line?

[balazsorban44]

https://nodejs.org/api/url.html#url_class_url

If I understand this correctly, URL is supported since Node 10, which is the lowest supported node version anyway, so I think it is safe to use now. Am I wrong?

[Zertz]

Good point! In browsers however, people who need to support Internet Explorer would have to include a polyfill.

@JohnProto Perhaps the original author had a reason to do it this way? Otherwise, I agree the ? doesn't seem to serve a purpose 🤔

[balazsorban44]

Next.js includes polyfills for IE11, I assume URL is no different. Or wouldn't that cut it, even though this package will always be used together with next?

[github-actions]

🎉 This issue has been resolved in version 3.2.0-canary.34 🎉

The release is available on:

• GitHub release
• npm package (@canary dist-tag)

Your semantic-release bot 📦🚀

[github-actions]

🎉 This issue has been resolved in version 3.2.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀