Skip to content

Commit 0ee14d5

Browse files
committed
url: drop ICU requirement for parsing hostnames
PR-URL: nodejs#47339 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 21f23da commit 0ee14d5

File tree

5 files changed

+22
-66
lines changed

5 files changed

+22
-66
lines changed

deps/ada/ada.gyp

+2-17
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{
22
'variables': {
33
'v8_enable_i18n_support%': 1,
4+
'ada_sources': [ 'ada.cpp' ],
45
},
56
'targets': [
67
{
@@ -10,23 +11,7 @@
1011
'direct_dependent_settings': {
1112
'include_dirs': ['.'],
1213
},
13-
'sources': ['ada.cpp'],
14-
'conditions': [
15-
['v8_enable_i18n_support==0', {
16-
'defines': ['ADA_HAS_ICU=0'],
17-
}],
18-
['v8_enable_i18n_support==1', {
19-
'dependencies': [
20-
'<(icu_gyp_path):icui18n',
21-
'<(icu_gyp_path):icuuc',
22-
],
23-
}],
24-
['OS=="win" and v8_enable_i18n_support==1', {
25-
'dependencies': [
26-
'<(icu_gyp_path):icudata',
27-
],
28-
}],
29-
]
14+
'sources': [ '<@(ada_sources)' ]
3015
},
3116
]
3217
}

doc/api/url.md

+15-10
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ return `true`.
129129

130130
#### `new URL(input[, base])`
131131

132+
<!--
133+
changes:
134+
- version: v20.0.0
135+
pr-url: https://github.com/nodejs/node/pull/47339
136+
description: ICU requirement is removed.
137+
-->
138+
132139
* `input` {string} The absolute or relative input URL to parse. If `input`
133140
is relative, then `base` is required. If `input` is absolute, the `base`
134141
is ignored. If `input` is not a string, it is [converted to a string][] first.
@@ -172,9 +179,6 @@ const myURL = new URL('https://測試');
172179
// https://xn--g6w251d/
173180
```
174181

175-
This feature is only available if the `node` executable was compiled with
176-
[ICU][] enabled. If not, the domain names are passed through unchanged.
177-
178182
In cases where it is not known in advance if `input` is an absolute URL
179183
and a `base` is provided, it is advised to validate that the `origin` of
180184
the `URL` object is what is expected.
@@ -1008,6 +1012,10 @@ for (const [name, value] of params) {
10081012
added:
10091013
- v7.4.0
10101014
- v6.13.0
1015+
changes:
1016+
- version: REPLACEME
1017+
pr-url: https://github.com/nodejs/node/pull/47339
1018+
description: ICU requirement is removed.
10111019
-->
10121020

10131021
* `domain` {string}
@@ -1018,9 +1026,6 @@ invalid domain, the empty string is returned.
10181026

10191027
It performs the inverse operation to [`url.domainToUnicode()`][].
10201028

1021-
This feature is only available if the `node` executable was compiled with
1022-
[ICU][] enabled. If not, the domain names are passed through unchanged.
1023-
10241029
```mjs
10251030
import url from 'node:url';
10261031

@@ -1049,6 +1054,10 @@ console.log(url.domainToASCII('xn--iñvalid.com'));
10491054
added:
10501055
- v7.4.0
10511056
- v6.13.0
1057+
changes:
1058+
- version: REPLACEME
1059+
pr-url: https://github.com/nodejs/node/pull/47339
1060+
description: ICU requirement is removed.
10521061
-->
10531062

10541063
* `domain` {string}
@@ -1059,9 +1068,6 @@ domain, the empty string is returned.
10591068

10601069
It performs the inverse operation to [`url.domainToASCII()`][].
10611070

1062-
This feature is only available if the `node` executable was compiled with
1063-
[ICU][] enabled. If not, the domain names are passed through unchanged.
1064-
10651071
```mjs
10661072
import url from 'node:url';
10671073

@@ -1699,7 +1705,6 @@ console.log(myURL.origin);
16991705
// Prints https://xn--1xa.example.com
17001706
```
17011707
1702-
[ICU]: intl.md#options-for-building-nodejs
17031708
[Punycode]: https://tools.ietf.org/html/rfc5891#section-4.4
17041709
[WHATWG URL]: #the-whatwg-url-api
17051710
[WHATWG URL Standard]: https://url.spec.whatwg.org/

lib/internal/idna.js

+2-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
11
'use strict';
22

3-
if (internalBinding('config').hasIntl) {
4-
const { toASCII, toUnicode } = internalBinding('icu');
5-
module.exports = { toASCII, toUnicode };
6-
} else {
7-
const { domainToASCII, domainToUnicode } = require('internal/url');
8-
module.exports = { toASCII: domainToASCII, toUnicode: domainToUnicode };
9-
}
3+
const { domainToASCII, domainToUnicode } = require('internal/url');
4+
module.exports = { toASCII: domainToASCII, toUnicode: domainToUnicode };

test/benchmark/test-benchmark-url.js

+1-13
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,6 @@
11
'use strict';
22

3-
const common = require('../common');
4-
5-
// TODO(@anonrig): Remove this check when Ada removes ICU requirement.
6-
if (!common.hasIntl) {
7-
// A handful of the benchmarks fail when ICU is not included.
8-
// ICU is responsible for ignoring certain inputs from the hostname
9-
// and without it, it is not possible to validate the correctness of the input.
10-
// DomainToASCII method in Unicode specification states which characters are
11-
// ignored and/or remapped. Doing this outside of the scope of DomainToASCII,
12-
// would be a violation of the WHATWG URL specification.
13-
// Please look into: https://unicode.org/reports/tr46/#ProcessingStepMap
14-
common.skip('missing Intl');
15-
}
3+
require('../common');
164

175
const runBenchmark = require('../common/benchmark');
186

test/wpt/status/url.json

+2-19
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,19 @@
11
{
2-
"toascii.window.js": {
3-
"requires": ["small-icu"]
4-
},
52
"percent-encoding.window.js": {
6-
"requires": ["small-icu"],
73
"skip": "TODO: port from .window.js"
84
},
95
"historical.any.js": {
10-
"requires": ["small-icu"],
116
"fail": {
127
"expected": [
138
"URL: no structured serialize/deserialize support",
149
"URLSearchParams: no structured serialize/deserialize support"
1510
]
1611
}
1712
},
18-
"urlencoded-parser.any.js": {
19-
"requires": ["small-icu"]
20-
},
21-
"url-constructor.any.js": {
22-
"requires": ["small-icu"]
23-
},
24-
"url-origin.any.js": {
25-
"requires": ["small-icu"]
26-
},
27-
"url-setters.any.js": {
28-
"requires": ["small-icu"]
29-
},
3013
"url-setters-a-area.window.js": {
3114
"skip": "already tested in url-setters.any.js"
3215
},
33-
"IdnaTestV2.window.js": {
34-
"requires": ["small-icu"]
16+
"javascript-urls.window.js": {
17+
"skip": "requires document.body reference"
3518
}
3619
}

0 commit comments

Comments
 (0)