Add support for relative protocols in url filter #1276
Conversation
|
This looks good to me. Although it's also potentially a breaking change for some people. I think we'd need to document and communicate this change thoroughly. @zachleat this is slightly off topic but it would be super useful if the website documentation was in the same repo as eleventy (possibility even a monorepo (...I know, I know, but you can do it with a light touch, it's not all bad!)) I also think using changeset could be a good addition to managing things: https://github.com/atlassian/changesets. Let me know if you'd be open to PRs for either of these. |
| // work with undefined | ||
| url = url || ""; | ||
|
|
||
| if ( | ||
| validUrl.isUri(url) || | ||
| url.indexOf("http://") === 0 || | ||
| url.indexOf("https://") === 0 | ||
| url.indexOf("https://") === 0 || | ||
| (url.indexOf("//") === 0 && url !== "//") |
There was a problem hiding this comment.
This could also be written as (url.startsWith("//") && url !== "//")
Actually all of them in the conditional can be replaced with startsWith() as there is no way the url to contain the protocol in any other position except the... start of the string
|
I think it should be mentioned that protocol relative URLs can be harmful, and are now considered an anti-pattern: |
zachleat
added
the
breaking-change
|
This will ship with 1.0 |
This PR fixes #1247
Currently, the url filter normalize values starting with
//as if they where paths, so for example a url to//example.comis changed to/example.com(orpathPrefix/example.comif there's a path prefix configured).But technically,
//example.comis a perfectly valid url that should be resolved tohttp://example.comorhttps://example.com, depending of the protocol of the current document location.This PR fixes this problem. Note that this is a breaking change.