ampproject / ampproject/amphtml

Move JS from url Parsing via `HTMLAnchorElement`s to URL Constructor

Open
#35,491 3 comments 0 reactions 0 assignees View on GitHub
P3: When Possible Stale Type: Feature Request WG: runtime
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

### Description

The URL constructor could be used as a replacement for parsing URLs using an `HTMLAnchorElement` now that IE support is going away.

However, there are a few difficult issues to solve since the codebase relies on invalid URL parsing by extended the intended meaning from the specification. In the case a URL resolves to a `'null'` origin, AMP currently overrides the value with the following logic.

```
// For data URI anchorEl.origin is equal to the string 'null' which is not useful.
// We instead return the actual origin which is the full URL.
let origin;
if (anchorEl.origin && anchorEl.origin != 'null') {
origin = anchorEl.origin;
} else if (info.protocol == 'data:' || !info.host) {
origin = info.href;
} else {
origin = info.protocol + '//' + info.host;
}
info.origin = origin;
```

This change means for instance, `data:12345` returns a value for the origin, even though this Location is invalid.

I made a series of changes to switch over to using the URL constructor as part of the initial PRs for IE Deprecation [here](https://github.com/ampproject/amphtml/pull/35317/commits).

Since the number of changes dedicated to URL construction and usage was greater than all other changes in the PR, this issue tracks the desire to attempt this change again in the future.

### Alternatives Considered

The project could choose to only use URL constructors in the `module` form of the libraries. However, this means the module form would potentially use the `'null'` key for uniqueness across origins in scenarios and unintentionally might use the same instance of a Service across two iframes (about:srcdoc returns a 'null' origin when using URL constructors).

### Additional Context

Additionally it might be worth considering a change to the types and methods used in URLs across the project. Right now, many methods accepts `{string|Location}` and coerce the value into a `Location` multiple times in a single frame of JS time.

Moving to an interface where only `Locations` are used by helpers could eliminate duplicative parsing per JS frame.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the URL-construction changes in PR #35317 and the existing helpers that parse URLs through HTMLAnchorElement. Define how URL construction should preserve AMP's handling of data URLs, null origins, invalid URLs, and iframe uniqueness before attempting the broader migration; done means the relevant parsing and Location interfaces consistently use the new approach without changing those behaviors.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
web-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.