Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0dc485e

Browse filesBrowse files
anonrigdanielleadams
anonrig
authored and
danielleadams
committed
url: drop ICU requirement for parsing hostnames
PR-URL: #47339 Backport-PR-URL: #48345 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent b395b16 commit 0dc485e
Copy full SHA for 0dc485e

File tree

Expand file treeCollapse file tree

5 files changed

+20
-68
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+20
-68
lines changed
Open diff view settings
Collapse file

‎deps/ada/ada.gyp‎

Copy file name to clipboard
+2-17Lines changed: 2 additions & 17 deletions
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
}
Collapse file

‎doc/api/url.md‎

Copy file name to clipboardExpand all lines: doc/api/url.md
+15-10Lines changed: 15 additions & 10 deletions
  • Display the source diff
  • Display the rich diff
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: REPLACEME
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.
@@ -1029,6 +1033,10 @@ for (const [name, value] of params) {
10291033
added:
10301034
- v7.4.0
10311035
- v6.13.0
1036+
changes:
1037+
- version: REPLACEME
1038+
pr-url: https://github.com/nodejs/node/pull/47339
1039+
description: ICU requirement is removed.
10321040
-->
10331041

10341042
* `domain` {string}
@@ -1039,9 +1047,6 @@ invalid domain, the empty string is returned.
10391047

10401048
It performs the inverse operation to [`url.domainToUnicode()`][].
10411049

1042-
This feature is only available if the `node` executable was compiled with
1043-
[ICU][] enabled. If not, the domain names are passed through unchanged.
1044-
10451050
```mjs
10461051
import url from 'node:url';
10471052

@@ -1070,6 +1075,10 @@ console.log(url.domainToASCII('xn--iñvalid.com'));
10701075
added:
10711076
- v7.4.0
10721077
- v6.13.0
1078+
changes:
1079+
- version: REPLACEME
1080+
pr-url: https://github.com/nodejs/node/pull/47339
1081+
description: ICU requirement is removed.
10731082
-->
10741083

10751084
* `domain` {string}
@@ -1080,9 +1089,6 @@ domain, the empty string is returned.
10801089

10811090
It performs the inverse operation to [`url.domainToASCII()`][].
10821091

1083-
This feature is only available if the `node` executable was compiled with
1084-
[ICU][] enabled. If not, the domain names are passed through unchanged.
1085-
10861092
```mjs
10871093
import url from 'node:url';
10881094

@@ -1725,7 +1731,6 @@ console.log(myURL.origin);
17251731
// Prints https://xn--1xa.example.com
17261732
```
17271733
1728-
[ICU]: intl.md#options-for-building-nodejs
17291734
[Punycode]: https://tools.ietf.org/html/rfc5891#section-4.4
17301735
[WHATWG URL]: #the-whatwg-url-api
17311736
[WHATWG URL Standard]: https://url.spec.whatwg.org/
Collapse file

‎lib/internal/idna.js‎

Copy file name to clipboard
+2-7Lines changed: 2 additions & 7 deletions
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 };
Collapse file

‎test/benchmark/test-benchmark-url.js‎

Copy file name to clipboardExpand all lines: test/benchmark/test-benchmark-url.js
+1-13Lines changed: 1 addition & 13 deletions
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

Collapse file

‎test/wpt/status/url.json‎

Copy file name to clipboard
-21Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +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"]
35-
},
3616
"javascript-urls.window.js": {
37-
"required": ["small-icu"],
3817
"skip": "requires document.body reference"
3918
}
4019
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.