Skip to content

url: make ICU-dependent functions warn for node compiled --without-intl

Rodrigo Muino Tomonari requested to merge github/fork/aduh95/url-icu-dependent-functions-throw into master

ICU dependent functions throw an ERR_NO_ICU error on Node.js versions compiled without ICU.

FYI the current behaviour is to return the input string unchanged when ICU is not available:

$ out/Release/node -p process.versions | grep icu # compiled --without-intl
$ out/Release/node -p "require('url').domainToUnicode('xn--wgv71a119e.com')"
xn--wgv71a119e.com
$ out/Release/node -p "require('url').domainToASCII('www.日本語.com')"
www.日本語.com

That's – to say the least – unexpected taht the function silently fails. This PR addresses that:

$ out/Release/node -p "require('url').domainToUnicode('xn--wgv71a119e.com')"
Warning: Cannot convert to ASCII when intl is disabled!
Warning: Cannot convert to unicode when intl is disabled!
xn--wgv71a119e.com
$ out/Release/node -p "require('url').domainToASCII('www.日本語.com')"
Warning: Cannot convert to ASCII when intl is disabled!
www.日本語.com
$
$ # This also affect WHATWG `URL`:
$
$ out/Release/node -e "new URL('http://www.日本語.com')"
Warning: Cannot convert to ASCII when intl is disabled!
$ out/Release/node -e "new URL('http://github.com')"    
Warning: Cannot convert to ASCII when intl is disabled!

This is a semver-major change.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports