Improve compatibility with uri-js

[gurgunday]

this test must pass if we want to merge fast-uri into ajv

I'm about to fix it, hence the PR

[mcollina]

lgtm

[mcollina]

Does this affect our benchmarks?

[zekth]

Is there a perf implication on this? It should have. Can't we use a different function based on env? Node vs browser?

[gurgunday]

Ok bench is not working, I will look into it later

Here are the results on my machine, but Izmir is burning right now (37° celsius) so my results aren't 100% accurate — but still, I don't observe a significant drop anywhere:

PR:

fast-uri: parse domain x 1,963,346 ops/sec ±1.99% (94 runs sampled)
urijs: parse domain x 725,481 ops/sec ±0.38% (101 runs sampled)
WHATWG URL: parse domain x 4,164,643 ops/sec ±0.16% (100 runs sampled)
fast-uri: parse IPv4 x 2,869,202 ops/sec ±0.20% (101 runs sampled)
urijs: parse IPv4 x 588,070 ops/sec ±0.39% (99 runs sampled)
fast-uri: parse IPv6 x 1,309,017 ops/sec ±0.61% (101 runs sampled)
urijs: parse IPv6 x 422,369 ops/sec ±0.98% (98 runs sampled)
fast-uri: parse URN x 3,504,515 ops/sec ±1.26% (96 runs sampled)
urijs: parse URN x 1,823,102 ops/sec ±0.51% (96 runs sampled)
WHATWG URL: parse URN x 4,845,320 ops/sec ±0.43% (100 runs sampled)
fast-uri: parse URN uuid x 2,305,687 ops/sec ±0.52% (96 runs sampled)
urijs: parse URN uuid x 1,287,622 ops/sec ±0.59% (97 runs sampled)
fast-uri: serialize uri x 1,768,425 ops/sec ±0.70% (90 runs sampled)
urijs: serialize uri x 577,715 ops/sec ±3.03% (97 runs sampled)
fast-uri: serialize IPv6 x 544,162 ops/sec ±0.86% (93 runs sampled)
urijs: serialize IPv6 x 383,924 ops/sec ±0.51% (95 runs sampled)
fast-uri: serialize ws x 1,783,879 ops/sec ±0.78% (95 runs sampled)
urijs: serialize ws x 557,360 ops/sec ±0.63% (97 runs sampled)
fast-uri: resolve x 496,207 ops/sec ±1.09% (92 runs sampled)
urijs: resolve x 322,312 ops/sec ±0.89% (94 runs sampled)

Main:

fast-uri: parse domain x 1,935,556 ops/sec ±0.60% (96 runs sampled)
urijs: parse domain x 716,592 ops/sec ±0.57% (97 runs sampled)
WHATWG URL: parse domain x 4,050,297 ops/sec ±0.91% (97 runs sampled)
fast-uri: parse IPv4 x 2,752,662 ops/sec ±0.41% (97 runs sampled)
urijs: parse IPv4 x 594,033 ops/sec ±0.57% (96 runs sampled)
fast-uri: parse IPv6 x 1,314,395 ops/sec ±0.47% (97 runs sampled)
urijs: parse IPv6 x 399,563 ops/sec ±0.75% (97 runs sampled)
fast-uri: parse URN x 3,378,312 ops/sec ±0.75% (96 runs sampled)
urijs: parse URN x 1,716,716 ops/sec ±1.06% (93 runs sampled)
WHATWG URL: parse URN x 4,666,504 ops/sec ±0.67% (97 runs sampled)
fast-uri: parse URN uuid x 2,283,147 ops/sec ±0.83% (98 runs sampled)
urijs: parse URN uuid x 1,227,278 ops/sec ±0.89% (93 runs sampled)
fast-uri: serialize uri x 1,264,827 ops/sec ±0.71% (91 runs sampled)
urijs: serialize uri x 556,325 ops/sec ±0.98% (91 runs sampled)
fast-uri: serialize IPv6 x 484,639 ops/sec ±0.99% (91 runs sampled)
urijs: serialize IPv6 x 351,026 ops/sec ±1.50% (93 runs sampled)
fast-uri: serialize ws x 1,296,084 ops/sec ±1.59% (92 runs sampled)
urijs: serialize ws x 520,719 ops/sec ±0.97% (87 runs sampled)
fast-uri: resolve x 407,145 ops/sec ±1.02% (88 runs sampled)
urijs: resolve x 304,759 ops/sec ±0.62% (90 runs sampled)

[Eomm]

Could you elaborate this change?

These functions are deprecated:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURI?retiredLocale=it
• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/unescape

> escape(unescape('шеллы'))
'%u0448%u0435%u043B%u043B%u044B'
> encodeURI('шеллы')
'%D1%88%D0%B5%D0%BB%D0%BB%D1%8B'

[gurgunday]

Hey,

So I want to start by saying that I fully agree with you in that deprecated functions should be avoided in general, but they have different behavior as you pointed out and this change is to achieve url-js compatibility (one of the goals of this library) — you can see that escape and unescape have already been used many times in this file:

fast-uri/index.js

Lines 265 to 274 in bf03d44

	if (!schemeHandler || (schemeHandler && !schemeHandler.skipNormalize)) {
	if (gotEncoding && parsed.scheme !== undefined) {
	parsed.scheme = unescape(parsed.scheme)
	}
	if (gotEncoding && parsed.userinfo !== undefined) {
	parsed.userinfo = unescape(parsed.userinfo)
	}
	if (gotEncoding && parsed.host !== undefined) {
	parsed.host = unescape(parsed.host)
	}

In fact, if I recall correctly, someone tried to change these functions to encode/decodeURI, and maybe the tests didn't catch the behavior change for path

Having said all that, I don't think these functions will ever be removed from the Web

[zekth]

✌️

[gurgunday]

Be 100% compatible with uri-js

Because we have this in the Todos/Goals, I feel it should be bugfix/patch release

Does anyone object?

Cc @fastify/libraries @fastify/plugins

[jasoniangreen]

Thanks all, I will prepare a new release of AJV with fast-uri. Let's try and fix forward if anything else comes up.

[mcollina]

Absolutely!