vndb/mithril-vndb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clarify pathname docs, follow spec with fragments (#2448)

Browse source 
* Clarify pathname docs, follow spec with fragments

- Valid URLs must not contain a `#` within its fragment.
  https://github.com/MithrilJS/mithril.js/issues/2445
- Our docs were a little confusing and misleading - `m.pathname` isn't
  aware of URLs, just path names.
- Removed the relevant extension to `m.parseQueryString` required to
  support the hash parsing extension. Now we just shave it off and
  ignore it.
- Fix support for arbitrary prefixes, so prefixes like `?#` are
  handled correctly.
- Add a bunch of tests to cover various areas of confusion and unusual
  edge cases.

* Update with PR [skip ci]
This commit is contained in:
Isiah Meadows 2019-07-03 06:22:25 -04:00 • committed by GitHub
parent 9e9b89d900
commit 85bfd0f77d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 85 additions and 127 deletions
Download patch file Download diff file Expand all files Collapse all files

3
docs/change-log.md
View file

@ -43,6 +43,8 @@
- request: set `Content-Type: application/json; charset=utf-8` for all XHR methods by default, provided they have a body that's `!= null` ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361), [#2421](https://github.com/MithrilJS/mithril.js/pull/2421))
- This can cause CORS issues when issuing `GET` with bodies, but you can address them through configuring CORS appropriately.
- Previously, it was only set for all non-`GET` methods and only when `useBody: true` was passed (the default), and it was always set for them. Now it's automatically omitted when no body is present, so the hole is slightly broadened.
- route: query parameters in hash strings are no longer supported ([#2448](https://github.com/MithrilJS/mithril.js/pull/2448) [@isiahmeadows](https://github.com/isiahmeadows))
- It's technically invalid in hashes, so I'd rather push people to keep in line with spec.
#### News
@ -96,6 +98,7 @@
- request: autoredraw support fixed for `async`/`await` in Chrome ([#2428](https://github.com/MithrilJS/mithril.js/pull/2428) [@isiahmeadows](https://github.com/isiahmeadows))
- render: fix when attrs change with `onbeforeupdate` returning false, then remaining the same on next redraw ([#2447](https://github.com/MithrilJS/mithril.js/pull/2447) [@isiahmeadows](https://github.com/isiahmeadows))
- render: fix internal error when `onbeforeupdate` returns false and then true with new child tree ([#2447](https://github.com/MithrilJS/mithril.js/pull/2447) [@isiahmeadows](https://github.com/isiahmeadows))
- route: arbitrary prefixes are properly supported now, including odd prefixes like `?#` and invalid prefixes like `#foo#bar` ([#2448](https://github.com/MithrilJS/mithril.js/pull/2448) [@isiahmeadows](https://github.com/isiahmeadows))
---

10
docs/parsePathname.md
View file

@ -32,11 +32,15 @@ Argument | Type | Required | Description
### How it works
The `m.parsePathname` method creates an object from a path with a possible query string and hash string. It is useful for parsing a URL into more sensible paths, and it's what [`m.route`](route.md) uses internally to normalize paths to later match them. It uses [`m.parseQueryString`](parseQueryString.md) to parse the query parameters into an object.
The `m.parsePathname` method creates an object from a path with a possible query string. It is useful for parsing a local path name into its parts, and it's what [`m.route`](route.md) uses internally to normalize paths to later match them. It uses [`m.parseQueryString`](parseQueryString.md) to parse the query parameters into an object.
```javascript
var data = m.parsePathname("/path/user?a=hello&b=world#random=hash&some=value")
var data = m.parsePathname("/path/user?a=hello&b=world")
// data.path is "/path/user"
// data.params is {a: "hello", b: "world", random: "hash", some: "value"}
// data.params is {a: "hello", b: "world"}
```
### General-purpose URL parsing
The method is called `parsePathname` because it applies to pathnames. If you want a general-purpose URL parser, you should use [the global `URL` class](https://developer.mozilla.org/en-US/docs/Web/API/URL) instead.

5
docs/parseQueryString.md
View file

@ -19,13 +19,12 @@ var object = m.parseQueryString("a=1&b=2")
### Signature
`object = m.parseQueryString(string, object)`
`object = m.parseQueryString(string)`
Argument | Type | Required | Description
------------ | ------------------------------------------ | -------- | ---
`string` | `String` | Yes | A querystring
`object` | `Object` | No | An existing key-value map to merge values into, potentially from a previous `m.parseQueryString` call
**returns** | `Object` | | A key-value map, `object` if provided
**returns** | `Object` | | A key-value map
[How to read signatures](signatures.md)

2
docs/paths.md
View file

@ -123,7 +123,7 @@ m.route(document.body, "/edit/1", {
})
```
Note that query parameters are implicit - you don't need to name them to accept them. You can match based on an existing value, like in `"/edit?type=image"`, but you don't need to use `"/edit?type=:type"` to accept the value. In fact, Mithril would treat that as you trying to literally match against `m.route.param("type") === ":type"`. Or in summary, use `m.route.param("key")` to extract parameters - it simplifies things.
Query parameters are implicitly consumed - you don't need to name them to accept them. You can match based on an existing value, like in `"/edit?type=image"`, but you don't need to use `"/edit?type=:type"` to accept the value. In fact, Mithril would treat that as you trying to literally match against `m.route.param("type") === ":type"`, so you probably don't want to do that. In short, use `m.route.param("key")` or route component attributes to read query parameters.
### Path normalization

12
pathname/parse.js
View file

@ -9,16 +9,16 @@ module.exports = function(url) {
var queryEnd = hashIndex < 0 ? url.length : hashIndex
var pathEnd = queryIndex < 0 ? queryEnd : queryIndex
var path = url.slice(0, pathEnd).replace(/\/{2,}/g, "/")
var params = {}
if (!path) path = "/"
else {
if (path[0] !== "/") path = "/" + path
if (path.length > 1 && path[path.length - 1] === "/") path = path.slice(0, -1)
}
// Note: these are reversed because `parseQueryString` appends parameters
// only if they don't exist. Please don't flip them.
if (queryIndex >= 0) parseQueryString(url.slice(queryIndex + 1, queryEnd), params)
if (hashIndex >= 0) parseQueryString(url.slice(hashIndex + 1), params)
return {path: path, params: params}
return {
path: path,
params: queryIndex < 0
? {}
: parseQueryString(url.slice(queryIndex + 1, queryEnd)),
}
}

12
pathname/tests/test-compileTemplate.js
View file

@ -198,18 +198,6 @@ o.spec("compileTemplate", function() {
o(compileTemplate("/route/:id?bar=foo")(data)).equals(false)
o(data.params).deepEquals({foo: "bar"})
})
o("checks hash params match", function() {
var data = parsePathname("/route/1#foo=bar")
o(compileTemplate("/route/:id#foo=bar")(data)).equals(true)
o(data.params).deepEquals({id: "1", foo: "bar"})
})
o("checks hash params mismatch", function() {
var data = parsePathname("/route/1#foo=bar")
o(compileTemplate("/route/:id#foo=1")(data)).equals(false)
o(data.params).deepEquals({foo: "bar"})
o(compileTemplate("/route/:id#bar=foo")(data)).equals(false)
o(data.params).deepEquals({foo: "bar"})
})
o("checks dot before dot", function() {
var data = parsePathname("/file.test.png/edit")
o(compileTemplate("/:file.:ext/edit")(data)).equals(true)

38
pathname/tests/test-parsePathname.js
View file

@ -18,18 +18,18 @@ o.spec("parsePathname", function() {
params: {a: "b", c: "d"}
})
})
o("parses hash at start", function() {
o("ignores hash at start", function() {
var data = parsePathname("#a=b&c=d")
o(data).deepEquals({
path: "/",
params: {a: "b", c: "d"}
params: {}
})
})
o("parses query + hash at start", function() {
o("parses query, ignores hash at start", function() {
var data = parsePathname("?a=1&b=2#c=3&d=4")
o(data).deepEquals({
path: "/",
params: {a: "1", b: "2", c: "3", d: "4"}
params: {a: "1", b: "2"}
})
})
o("parses root", function() {
@ -46,18 +46,18 @@ o.spec("parsePathname", function() {
params: {a: "b", c: "d"}
})
})
o("parses root + hash at start", function() {
o("parses root, ignores hash at start", function() {
var data = parsePathname("/#a=b&c=d")
o(data).deepEquals({
path: "/",
params: {a: "b", c: "d"}
params: {}
})
})
o("parses root + query + hash at start", function() {
o("parses root + query, ignores hash at start", function() {
var data = parsePathname("/?a=1&b=2#c=3&d=4")
o(data).deepEquals({
path: "/",
params: {a: "1", b: "2", c: "3", d: "4"}
params: {a: "1", b: "2"}
})
})
o("parses route", function() {
@ -102,25 +102,25 @@ o.spec("parsePathname", function() {
params: {c: "3", d: "4"}
})
})
o("parses route + query + hash", function() {
o("parses route + query, ignores hash", function() {
var data = parsePathname("/route/foo?a=1&b=2#c=3&d=4")
o(data).deepEquals({
path: "/route/foo",
params: {a: "1", b: "2", c: "3", d: "4"}
params: {a: "1", b: "2"}
})
})
o("deduplicates same-named params in query + hash", function() {
var data = parsePathname("/route/foo?a=1&b=2#a=3&c=4")
o(data).deepEquals({
path: "/route/foo",
params: {a: "3", b: "2", c: "4"}
})
})
o("parses route + query + hash with lots of junk slashes", function() {
o("parses route + query, ignores hash with lots of junk slashes", function() {
var data = parsePathname("//route/////foo//?a=1&b=2#c=3&d=4")
o(data).deepEquals({
path: "/route/foo",
params: {a: "1", b: "2", c: "3", d: "4"}
params: {a: "1", b: "2"}
})
})
o("doesn't comprehend protocols", function() {
var data = parsePathname("https://example.com/foo/bar")
o(data).deepEquals({
path: "/https:/example.com/foo/bar",
params: {}
})
})
})

5
querystring/parse.js
View file

@ -2,12 +2,11 @@
// The extra `data` parameter is for if you want to append to an existing
// parameters object.
module.exports = function(string, data) {
if (data == null) data = {}
module.exports = function(string) {
if (string === "" || string == null) return {}
if (string.charAt(0) === "?") string = string.slice(1)
var entries = string.split("&"), counters = {}
var entries = string.split("&"), counters = {}, data = {}
for (var i = 0; i < entries.length; i++) {
var entry = entries[i].split("=")
var key = decodeURIComponent(entry[0])

12
querystring/tests/test-parseQueryString.js
View file

@ -97,16 +97,4 @@ o.spec("parseQueryString", function() {
var data = parseQueryString("a=1&b=2&a=3")
o(data).deepEquals({a: "3", b: "2"})
})
o("continues to append to arrays between calls", function() {
var data = {}
parseQueryString("a[]=1&a[]=2", data)
parseQueryString("a[]=3&a[]=4", data)
o(data).deepEquals({a: ["1", "2", "3", "4"]})
})
o("continues to append to objects between calls", function() {
var data = {}
parseQueryString("a[b]=1&a[c]=2", data)
parseQueryString("a[d]=3&a[e]=4", data)
o(data).deepEquals({a: {b: "1", c: "2", d: "3", e: "4"}})
})
})

11
request/tests/test-request.js
View file

@ -73,6 +73,17 @@ o.spec("request", function() {
o(data).deepEquals({a: 1})
}).then(done)
})
o("first argument keeps protocol", function(done) {
mock.$defineRoutes({
"POST /item": function(request) {
o(request.rawUrl).equals("https://example.com/item")
return {status: 200, responseText: JSON.stringify({a: 1})}
}
})
request("https://example.com/item", {method: "POST"}).then(function(data) {
o(data).deepEquals({a: 1})
}).then(done)
})
o("works w/ parameterized data via GET", function(done) {
mock.$defineRoutes({
"GET /item": function(request) {

25
router/router.js
View file

@ -9,18 +9,25 @@ module.exports = function($window) {
var supportsPushState = typeof $window.history.pushState === "function"
var callAsync = typeof setImmediate === "function" ? setImmediate : setTimeout
function normalize(fragment) {
var data = $window.location[fragment].replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent)
if (fragment === "pathname" && data[0] !== "/") data = "/" + data
return data
}
var asyncId
var router = {prefix: "#!"}
router.getPath = function() {
if (router.prefix.charAt(0) === "#") return normalize("hash").slice(router.prefix.length)
if (router.prefix.charAt(0) === "?") return normalize("search").slice(router.prefix.length) + normalize("hash")
return normalize("pathname").slice(router.prefix.length) + normalize("search") + normalize("hash")
// Consider the pathname holistically. The prefix might even be invalid,
// but that's not our problem.
var prefix = $window.location.hash
if (router.prefix[0] !== "#") {
prefix = $window.location.search + prefix
if (router.prefix[0] !== "?") {
prefix = $window.location.pathname + prefix
if (prefix[0] !== "/") prefix = "/" + prefix
}
}
// This seemingly useless `.concat()` speeds up the tests quite a bit,
// since the representation is consistently a relatively poorly
// optimized cons string.
return prefix.concat()
.replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent)
.slice(router.prefix.length)
}
router.setPath = function(path, data, options) {

61
router/tests/test-defineRoutes.js
View file

@ -7,7 +7,7 @@ var Router = require("../../router/router")
o.spec("Router.defineRoutes", function() {
void [{protocol: "http:", hostname: "localhost"}, {protocol: "file:", hostname: "/"}].forEach(function(env) {
void ["#", "?", "", "#!", "?!", "/foo"].forEach(function(prefix) {
void ["#", "?", "", "#!", "?!", "/foo", "?#", "##"].forEach(function(prefix) {
o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() {
var $window, router, onRouteChange, onFail
@ -44,12 +44,12 @@ o.spec("Router.defineRoutes", function() {
})
o("resolves to route w/ escaped unicode", function(done) {
$window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6"
$window.location.href = prefix + "/%C3%B6?%C3%B6=%C3%B6"
router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail)
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö#ö=ö", "/ö"])
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö", "/ö"])
o(onFail.callCount).equals(0)
done()
@ -57,12 +57,12 @@ o.spec("Router.defineRoutes", function() {
})
o("resolves to route w/ unicode", function(done) {
$window.location.href = prefix + "/ö?ö=ö#ö=ö"
$window.location.href = prefix + "/ö?ö=ö"
router.defineRoutes({"/ö": {data: 2}}, onRouteChange, onFail)
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö#ö=ö", "/ö"])
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö", "/ö"])
o(onFail.callCount).equals(0)
done()
@ -138,45 +138,6 @@ o.spec("Router.defineRoutes", function() {
})
})
o("handles route with hash", function(done) {
$window.location.href = prefix + "/test#a=b&c=d"
router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "b", c: "d"}, "/test#a=b&c=d", "/test"])
o(onFail.callCount).equals(0)
done()
})
})
o("handles route with search and hash", function(done) {
$window.location.href = prefix + "/test?a=b#c=d"
router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "b", c: "d"}, "/test?a=b#c=d", "/test"])
o(onFail.callCount).equals(0)
done()
})
})
o("handles route with search and hash + duplicate params", function(done) {
$window.location.href = prefix + "/test?a=b#a=d"
router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "d"}, "/test?a=b#a=d", "/test"])
o(onFail.callCount).equals(0)
done()
})
})
o("calls reject", function(done) {
$window.location.href = prefix + "/test"
router.defineRoutes({"/other": {data: 1}}, onRouteChange, onFail)
@ -189,18 +150,6 @@ o.spec("Router.defineRoutes", function() {
})
})
o("calls reject w/ search and hash", function(done) {
$window.location.href = prefix + "/test?a=b#c=d"
router.defineRoutes({"/other": {data: 1}}, onRouteChange, onFail)
callAsync(function() {
o(onFail.callCount).equals(1)
o(onFail.args).deepEquals(["/test?a=b#c=d", {a: "b", c: "d"}])
done()
})
})
o("handles out of order routes", function(done) {
$window.location.href = prefix + "/z/y/x"
router.defineRoutes({"/z/y/x": {data: 1}, "/:a...": {data: 2}}, onRouteChange, onFail)

2
router/tests/test-getPath.js
View file

@ -6,7 +6,7 @@ var Router = require("../../router/router")
o.spec("Router.getPath", function() {
void [{protocol: "http:", hostname: "localhost"}, {protocol: "file:", hostname: "/"}].forEach(function(env) {
void ["#", "?", "", "#!", "?!", "/foo"].forEach(function(prefix) {
void ["#", "?", "", "#!", "?!", "/foo", "?#", "##"].forEach(function(prefix) {
o.spec("using prefix `" + prefix + "` starting on " + env.protocol + "//" + env.hostname, function() {
var $window, router, onRouteChange, onFail

11
test-utils/tests/test-parseURL.js
View file

@ -7,7 +7,7 @@ o.spec("parseURL", function() {
var root = {protocol: "http:", hostname: "localhost", port: "", pathname: "/"}
o.spec("full URL", function() {
o("parses full URL", function() {
o("parses full http URL", function() {
var data = parseURL("http://www.google.com:80/test?a=b#c")
o(data.protocol).equals("http:")
o(data.hostname).equals("www.google.com")
@ -16,6 +16,15 @@ o.spec("parseURL", function() {
o(data.search).equals("?a=b")
o(data.hash).equals("#c")
})
o("parses full websocket URL", function() {
var data = parseURL("ws://www.google.com:80/test?a=b#c")
o(data.protocol).equals("ws:")
o(data.hostname).equals("www.google.com")
o(data.port).equals("80")
o(data.pathname).equals("/test")
o(data.search).equals("?a=b")
o(data.hash).equals("#c")
})
o("parses full URL omitting optionals", function() {
var data = parseURL("http://www.google.com")
o(data.protocol).equals("http:")

3
test-utils/xhrMock.js
View file

@ -36,6 +36,7 @@ module.exports = function() {
}
this.open = function(method, url, async, user, password) {
var urlData = parseURL(url, {protocol: "http:", hostname: "localhost", port: "", pathname: "/"})
args.rawUrl = url
args.method = method
args.pathname = urlData.pathname
args.search = urlData.search
@ -56,7 +57,7 @@ module.exports = function() {
var self = this
if(!aborted) {
var handler = routes[args.method + " " + args.pathname] || serverErrorHandler.bind(null, args.pathname)
var data = handler({url: args.pathname, query: args.search || {}, body: body || null})
var data = handler({rawUrl: args.rawUrl, url: args.pathname, query: args.search || {}, body: body || null})
self.status = data.status
// Match spec
if (self.responseType === "json") {