Add URL #1359

kitsonk · 2018-12-17T08:09:32Z

Fixes part of: #1047

This PR adds the URL global object which makes it easier to parse URL and work with their parts. The code tries to functionally replicate the Web Standard but doesn't follow the logic of the standard (which has a lot of obtuse and obscure logic in the IDL).

LinusU · 2018-12-17T09:13:03Z

What do you think about adding the following test, which passes in Safari 12.0.2 and Node.js 11.1.0, but which I think will fail here since the value of hash is encodeURIComponent-ed on the way in, but not decoded on the way out.

test(function urlBaseString() {
  const url = new URL("http://example.com");
  url.hash = "%test";
  assertEqual(url.hash, "#%test");
  url.hash = url.hash;
  assertEqual(url.hash, "#%test");
});

ry

A clean new URL implementation - awesome! Looks good - but please add the test @LinusU suggests.

ry · 2018-12-17T16:56:56Z

js/url.ts

+];
+
+function parse(url: string): URLParts | undefined {
+  const urlMatch = urlRegExp.exec(url);


Do other URL implementations use RegExp?

Node.js uses a combination of splitting strings and regular expressions. It's url is a slightly different beast to the the whatwg URL. JSDom follows the IDL pedantically, so it ingests each character and does loads of branching. While it complies 100% including the logic, it is a lot of code. There are two other polyfills that I found on npm, one ingests per char, the other uses regular expressions.

kitsonk · 2018-12-17T23:20:21Z

@LinusU thanks for the heads up. I also found a couple issues with the .pathname setter as well.

js/url.ts

ry

LGTM

- Upgrade V8 to 7.2.502.16 (denoland#1403) - Remove support for extensionless import (denoland#1396) - make stdout unbuffered (denoland#1355) - Implement `Body.formData` for fetch (denoland#1393) - Improve handling of non-coercable objects in assertEqual (denoland#1385) - Avoid fetch segfault on empty Uri (denoland#1394) - Expose deno.inspect (denoland#1378) - "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379) - Add illegal header name and value guards (denoland#1375) - Fix URLSearchParams set() and constructor() (denoland#1368) - Remove prebuilt v8 support (denoland#1369) - Enable jumbo build in release. (denoland#1362) - Add URL implementation (denoland#1359) - Add console.count and console.time (denoland#1358)

- "cargo build" support (denoland#1369 denoland#1296 denoland#1377 denoland#1379) - Remove support for extensionless import (denoland#1396) - Upgrade V8 to 7.2.502.16 (denoland#1403) - make stdout unbuffered (denoland#1355) - Implement `Body.formData` for fetch (denoland#1393) - Improve handling of non-coercable objects in assertEqual (denoland#1385) - Avoid fetch segfault on empty Uri (denoland#1394) - Expose deno.inspect (denoland#1378) - Add illegal header name and value guards (denoland#1375) - Fix URLSearchParams set() and constructor() (denoland#1368) - Remove prebuilt v8 support (denoland#1369) - Enable jumbo build in release. (denoland#1362) - Add URL implementation (denoland#1359) - Add console.count and console.time (denoland#1358) - runtime arg check `URLSearchParams` (denoland#1390)

- "cargo build" support (#1369 #1296 #1377 #1379) - Remove support for extensionless import (#1396) - Upgrade V8 to 7.2.502.16 (#1403) - make stdout unbuffered (#1355) - Implement `Body.formData` for fetch (#1393) - Improve handling of non-coercable objects in assertEqual (#1385) - Avoid fetch segfault on empty Uri (#1394) - Expose deno.inspect (#1378) - Add illegal header name and value guards (#1375) - Fix URLSearchParams set() and constructor() (#1368) - Remove prebuilt v8 support (#1369) - Enable jumbo build in release. (#1362) - Add URL implementation (#1359) - Add console.count and console.time (#1358) - runtime arg check `URLSearchParams` (#1390)

kitsonk force-pushed the url branch from 3dcc54a to 9c33362 Compare December 17, 2018 08:42

ry reviewed Dec 17, 2018

View reviewed changes

kitsonk force-pushed the url branch from 9c33362 to 0c81e9d Compare December 17, 2018 22:08

eventualbuddha reviewed Dec 18, 2018

View reviewed changes

js/url.ts Outdated Show resolved Hide resolved

Add URL

cfce21b

kitsonk force-pushed the url branch from 0c81e9d to cfce21b Compare December 18, 2018 00:33

ry approved these changes Dec 18, 2018

View reviewed changes

ry merged commit 91a41ca into denoland:master Dec 18, 2018

ry mentioned this pull request Dec 23, 2018

v0.2.4 #1405

Merged

kitsonk mentioned this pull request Jan 20, 2019

Compatibility with WHATWG at URL and URLSearchParams #1551

Closed

kitsonk deleted the url branch August 2, 2022 04:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add URL #1359

Add URL #1359

kitsonk commented Dec 17, 2018

LinusU commented Dec 17, 2018

ry left a comment

ry Dec 17, 2018

kitsonk Dec 17, 2018

kitsonk commented Dec 17, 2018

ry left a comment

Add URL #1359

Add URL #1359

Conversation

kitsonk commented Dec 17, 2018

LinusU commented Dec 17, 2018

ry left a comment

Choose a reason for hiding this comment

ry Dec 17, 2018

Choose a reason for hiding this comment

kitsonk Dec 17, 2018

Choose a reason for hiding this comment

kitsonk commented Dec 17, 2018

ry left a comment

Choose a reason for hiding this comment