Skip to content

Commit

Permalink
Streamline route/request path handling, split params + body in requests
Browse files Browse the repository at this point in the history
Fixes #2360
Fixes #1138
Fixes #1788 a little less hackishly
Probably fixes a few other issues I'm not aware of.

This more or less goes with @lhorie's comment here, just with a minor name
change from `query` to `params`:

#1138 (comment)

Specifically, here's what this patch entails:

- I changed `data` and `useBody` to `params` and `body` in `m.request`.
  Migration is trivial: just use `params` or `body` depending on which you
  intend to send. Most servers do actually care where the data goes, so you can
  generally pretty easily translate this accordingly. If you *really* need the
  old behavior, pass the old value in `params` and if `method === "GET"` or
  `method === "TRACE"`, also in `body`.
- I opened up all methods to have request bodies.
- I fixed `m.parseQueryString` to prefer later values over earlier values and
  to ensure that objects and arrays are persisted across both hash and query
  param parsing. That method also accepts an existing key/value map to append
  to, to simplify deduplication.
- I normalized path interpolation to be identical between routes and requests.
- I no longer include interpolated values in query strings. If you need to
  duplicate values again, rename the interpolation to be a distinct property
  and pass the value you want to duplicate as it.
- I converted `m.route` to use pre-compiled routes instead of its existing
  system of dynamic runtime checking. This shouldn't have a major effect on
  performance short-term, but it'll ease the migration to built-in userland
  components and make it a little easier to reconcile. It'll also come handy
  for large numbers of routes.
- I added support for matching routes like `"/:file.:ext"` or
  `"/:lang-:region"`, giving each defined semantics.
- I added support for matching against routes with static query strings, such
  as `"/edit?type=image": { ... }`.
- I'm throwing a few new informative errors.
- And I've updated the docs accordingly.

I also made a few drive-by edits:

- I fixed a bug in the `Stream.HALT` warning where it warned all but the first
  usage when the intent was to warn only on first use.
- Some of the tests were erroneously using `Stream.HALT` when they should've
  been using `Stream.SKIP`. I've fixed the tests to only test that
  `Stream.HALT === Stream.SKIP` and that it only warns on first use.
- The `m.request` and `m.jsonp` docs signatures were improved to more clearly
  explain how `m.request(url, options?)` and `m.jsonp(url, options?)` translate
  to `m.request(options)` and `m.jsonp(options)` respectively.

-----

There is some justification to these changes:

- In general, it matters surprisingly more than you would expect how things
  translate to HTTP requests. So the comment there suggesting a thing that
  papers over the difference has led to plenty of confusion in both Gitter and
  in GitHub issues.

- A lot of servers expect a GET with a body and no parameters, and leaving
  `m.request` open to working with that makes it much more flexible.

- Sometimes, servers expect a POST with query parameters *instead* of a JSON
  object. I've seen this quite a bit, even with more popular REST APIs like
  Stack Overflow's.

- I've encountered a few servers that expect both parameters and a body, each
  with distinct semantic meaning, so the separation makes it much easier to
  translate into a request.

- Most of the time, path segments are treated individually, and URL-escaping
  the contents is much less error-prone. It also avoids being potentially
  lossy, and when the variable in question isn't trusted, escaping the path
  segment enables you to pass it through the URL and not risk being redirected
  to unexpected locations, avoiding some risks of vulnerabilities and client
  side crashes.

If you really don't care how the template and parameters translate to an
eventual URL, just pass the same object for the `params` and `body` and use
`:param...` for each segment. Either way, the more explicit nature should help
a lot in making the intent clearer, whether you care or not.
  • Loading branch information
dead-claudia committed Feb 7, 2019
1 parent a886520 commit c5c5698
Show file tree
Hide file tree
Showing 28 changed files with 1,068 additions and 168 deletions.
4 changes: 2 additions & 2 deletions api/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ module.exports = function($window, redrawService) {
if (payload.onmatch) {
Promise.resolve(payload.onmatch(params, path)).then(function(resolved) {
update(payload, resolved)
}, bail)
}, function () { bail(path) })
}
else update(payload, "div")
}
}, bail)
}, bail, defaultRoute)
}
route.set = function(path, data, options) {
if (lastUpdate != null) {
Expand Down
3 changes: 2 additions & 1 deletion api/tests/test-router.js
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,8 @@ o.spec("route", function() {
$window.location.href = prefix + "/a"
route(root, "/", {
"/a": {view: view},
"/b": {onmatch: onmatch}
"/b": {onmatch: onmatch},
"/": {view: function() {}}
})

o(view.callCount).equals(1)
Expand Down
41 changes: 41 additions & 0 deletions docs/buildPathname.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# buildPathname(object)

- [Description](#description)
- [Signature](#signature)
- [How it works](#how-it-works)

---

### Description

Turns a [path template](paths.md) and a parameters object into a string of form `/path/user?a=1&b=2`

```javascript
var querystring = m.buildPathname("/path/:id", {id: "user", a: "1", b: "2"})
// "/path/user?a=1&b=2"
```

---

### Signature

`querystring = m.buildPathname(object)`

Argument | Type | Required | Description
------------ | ------------------------------------------ | -------- | ---
`object` | `Object` | Yes | A key-value map to be converted into a string
**returns** | `String` | | A string representing the input object

[How to read signatures](signatures.md)

---

### How it works

The `m.buildPathname` creates a [path name](paths.md) from a path template and a parameters object. It's useful for building URLs, and it's what [`m.route`](route.md), [`m.request`](request.md), and [`m.jsonp`](jsonp.md) all use internally to interpolate paths. It uses [`m.buildQueryString`](buildQueryString.md) to generate the query parameters to append to the path name.

```javascript
var querystring = m.buildPathname("/path/:id", {id: "user", a: 1, b: 2})

// querystring is "/path/user?a=1&b=2"
```
8 changes: 8 additions & 0 deletions docs/change-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
- cast className using toString ([#2309](https://github.com/MithrilJS/mithril.js/pull/2309))
- render: call attrs' hooks first, with express exception of `onbeforeupdate` to allow attrs to block components from even diffing ([#2297](https://github.com/MithrilJS/mithril.js/pull/2297))
- API: `m.withAttr` removed. ([#2317](https://github.com/MithrilJS/mithril.js/pull/2317))
- request: `data` has now been split to `params` and `body` and `useBody` has been removed in favor of just using `body`. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))
- route, request: Interpolated arguments are URL-escaped (and for declared routes, URL-unescaped) automatically. If you want to use a raw route parameter, use a variadic parameter like in `/asset/:path.../view`. This was previously only available in `m.route` route definitions, but it's now usable in both that and where paths are accepted. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))
- route, request: Interpolated arguments are *not* appended to the query string. This means `m.request({url: "/api/user/:id/get", params: {id: user.id}})` would result in a request like `GET /api/user/1/get`, not one like `GET /api/user/1/get?id=1`. If you really need it in both places, pass the same value via two separate parameters with the non-query-string parameter renamed, like in `m.request({url: "/api/user/:urlID/get", params: {id: user.id, urlID: user.id}})`. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))
- route, request: `m.route.set`, `m.request`, and `m.jsonp` all use the same path template syntax now, and vary only in how they receive their parameters. Furthermore, declared routes in `m.route` shares the same syntax and semantics, but acts in reverse as if via pattern matching. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))

#### News

Expand All @@ -57,6 +61,10 @@
- `m()` itself from `mithril` is exported as the default export.
- `mithril/stream`'s primary export is exported as the default export.
- fragments: allow same attrs/children overloading logic as hyperscript ([#2328](https://github.com/MithrilJS/mithril.js/pull/2328))
- route: Declared routes may check against path names with query strings. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))
- route: Declared routes in `m.route` now support `-` and `.` as delimiters for path segments. This means you can have a route like `"/edit/:file.:ext"`. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))
- Previously, this was possible to do in `m.route.set`, `m.request`, and `m.jsonp`, but it was wholly untested for and also undocumented.
- API: `m.buildPathname` and `m.parsePathname` added. ([#2361](https://github.com/MithrilJS/mithril.js/pull/2361))

#### Bug fixes

Expand Down
17 changes: 13 additions & 4 deletions docs/jsonp.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,29 @@ m.jsonp({

### Signature

`promise = m.jsonp([url,] options)`
`promise = m.jsonp(options)`

Argument | Type | Required | Description
---------------------- | --------------------------------- | -------- | ---
`url` | `String` | No | If present, it's equivalent to having the option `{url: url}`. Values passed to the `options` argument override options set via this shorthand.
`options.url` | `String` | Yes | The URL to send the request to. The URL may be either absolute or relative, and it may contain [interpolations](#dynamic-urls).
`options` | `Object` | Yes | The request options to pass.
`options.url` | `String` | Yes | The [path name](paths.md) to send the request to, optionally interpolated with values from `options.data`.
`options.data` | `any` | No | The data to be interpolated into the URL and serialized into the querystring.
`options.type` | `any = Function(any)` | No | A constructor to be applied to each object in the response. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function).
`options.callbackName` | `String` | No | The name of the function that will be called as the callback. Defaults to a randomized string (e.g. `_mithril_6888197422121285_0({a: 1})`
`options.callbackKey` | `String` | No | The name of the querystring parameter name that specifies the callback name. Defaults to `callback` (e.g. `/someapi?callback=_mithril_6888197422121285_0`)
`options.background` | `Boolean` | No | If `false`, redraws mounted components upon completion of the request. If `true`, it does not. Defaults to `false`.
**returns** | `Promise` | | A promise that resolves to the response data, after it has been piped through `type` method

`promise = m.jsonp(url, options)`

Argument | Type | Required | Description
----------- | --------- | -------- | ---
`url` | `String` | Yes | The [path name](paths.md) to send the request to. `options.url` overrides this when present.
`options` | `Object` | No | The request options to pass.
**returns** | `Promise` | | A promise that resolves to the response data, after it has been piped through the `type` method

This second form is mostly equivalent to `m.jsonp(Object.assign({url: url}, options))`, just it does not depend on the ES6 global `Object.assign` internally.

[How to read signatures](signatures.md)

---
Expand Down Expand Up @@ -87,4 +97,3 @@ m.jsonp({
console.log(response.data.login) // logs "lhorie"
})
```

1 change: 1 addition & 0 deletions docs/nav-guides.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- [Testing](testing.md)
- [Examples](examples.md)
- [3rd Party Integration](integrating-libs.md)
- [Path Handling](paths.md)
- Key concepts
- [Vnodes](vnodes.md)
- [Components](components.md)
Expand Down
42 changes: 42 additions & 0 deletions docs/parsePathname.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# parsePathname(string)

- [Description](#description)
- [Signature](#signature)
- [How it works](#how-it-works)

---

### Description

Turns a string of the form `/path/user?a=1&b=2` to an object

```javascript
var object = m.parsePathname("/path/user?a=1&b=2")
// {path: "/path/user", params: {a: "1", b: "2"}}
```

---

### Signature

`object = m.parsePathname(string)`

Argument | Type | Required | Description
------------ | -------- | -------- | ---
`string` | `String` | Yes | A URL
**returns** | `Object` | | A `{path, params}` pair where `path` is the [normalized path](paths.md#path-normalization) and `params` is the [parsed parameters](paths.md#parameter-normalization).

[How to read signatures](signatures.md)

---

### 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.

```javascript
var data = m.parsePathname("/path/user?a=hello&b=world#random=hash&some=value")

// data.path is "/path/user"
// data.params is {a: "hello", b: "world", random: "hash", some: "value"}
```
7 changes: 4 additions & 3 deletions docs/parseQueryString.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ var object = m.parseQueryString("a=1&b=2")

### Signature

`object = m.parseQueryString(string)`
`object = m.parseQueryString(string, object)`

Argument | Type | Required | Description
------------ | ------------------------------------------ | -------- | ---
`string` | `String` | Yes | A querystring
**returns** | `Object` | | A key-value map
`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

[How to read signatures](signatures.md)

Expand Down Expand Up @@ -68,4 +69,4 @@ Querystrings that contain bracket notation are correctly parsed into deep data s
m.parseQueryString("a[0]=hello&a[1]=world")

// data is {a: ["hello", "world"]}
```
```
132 changes: 132 additions & 0 deletions docs/paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Path Handling

- [Path types](#path-types)
- [Path parameters](#path-parameters)
- [Parameter normalization](#parameter-normalization)
- [Path normalization](#path-normalization)

-----

[`m.route`](route.md), [`m.request`](request.md), and [`m.jsonp`](jsonp.md) each have a concept called a path. This is used to generate the URL you route to or fetch from.

### Path types

There are two general types of paths: raw paths and parameterized paths.

- Raw paths are simply strings used directly as URLs. Nothing is substituted or even split. It's just normalized with all the parameters appended to the end.
- Parameterized paths let you insert values into paths, escaped by default for convenience and safety against URL injection.

For [`m.request`](request.md) and [`m.jsonp`](jsonp.md), these can be pretty much any URL, but for [routes](route.md), these can only be absolute URL path names without schemes or domains.

### Path parameters

Path parameters are themselves pretty simple. They come in two forms:

- `:foo` - This injects a simple `params.foo` into the URL, escaping its value first.
- `:foo...` - This injects a raw `params.foo` path into the URL without escaping anything.

You're probably wondering what that `params` object is supposed to be. It's pretty simple: it's the `params` in either [`m.route.set(path, params)`](route.md#mrouteset), [`m.request({url, params})`](request.md#signature), or [`m.jsonp({url, params})`](jsonp.md#signature).

When receiving routes via [`m.route(root, defaultRoute, routes)`](route.md#signature), you can use these parameters to *extract* values from routes. They work basically the same way as generating the paths, just in the opposite direction.

```javascript
// Edit a single item
m.route(document.body, "/edit/1", {
"/edit/:id": {
view: function() {
return [
m(Menu),
m("h1", "Editing user " + m.route.param("id"))
]
}
},
})

// Edit an item identified by path
m.route(document.body, "/edit/pictures/image.jpg", {
"/edit/:file...": {
view: function() {
return [
m(Menu),
m("h1", "Editing file " + m.route.param("file"))
]
}
},
})
```

In the first example, assuming you're navigating to the default route in each, `m.route.param("id")` would be read as `"1"` and `m.route.param("file")` would be read as `pictures/image.jpg`.

Path parameters may be delimited by either a `/`, `-`, or `.`. This lets you have dynamic path segments, and they're considerably more flexible than just a path name. For example, you could match against routes like `"/edit/:name.:ext"` for editing based on file extension or `"/:lang-:region/view"` for a localized route.

Path parameters are greedy: given a declared route `"/edit/:name.:ext"`, if you navigate to `/edit/file.test.png`, the parameters extracted will be `{name: "file.test", ext: "png"}`, not `{name: "file", ext: "test.png"}`. Similarly, given `"/route/:path.../view/:child..."`, if you go to `/route/foo/view/bar/view/baz`, the parameters extracted will be `{path: "foo/view/bar", child: "baz"}`.

### Parameter normalization

Path parameters that are interpolated into path names are omitted from the query string, for convenience and to keep the path name reasonably readable. For example, this sends a server request of `GET /api/user/1/connections?sort=name-asc`, omitting the duplicate `id=1` in the URL string.

```javascript
m.request({
url: "https://example.com/api/user/:userID/connections",
params: {
userID: 1,
sort: "name-asc"
}
})
```

You can also specify parameters explicitly in the query string itself, such as in this, which is equivalent to the above:

```javascript
m.request({
url: "https://example.com/api/user/:userID/connections?sort=name-asc",
params: {
userID: 1
}
})
```

And of course, you can mix and match. This fires a request to `GET /api/user/1/connections?sort=name-asc&first=10`.

```javascript
m.request({
url: "https://example.com/api/user/:userID/connections?sort=name-asc",
params: {
userID: 1,
first: 10
}
})
```

This even extends to route matching: you can match against a route *with* explicit query strings. It retains the matched parameter for convenience, so you can still access them via vnode parameters or via [`m.route.param`](route.md#mrouteparam). Note that although this *is* possible, it's not generally recommended, since you should prefer paths for pages. It could sometimes useful if you need to generate a somewhat different view just for a particular file type, but it still logically is a query-like parameter, not a whole separate page.

```javascript
// Note: this is generally *not* recommended - you should prefer paths for route
// declarations, not query strings.
m.route(document.body, "/edit/1", {
"/edit?type=image": {
view: function() {
return [
m(Menu),
m("h1", "Editing photo")
]
}
},
"/edit": {
view: function() {
return [
m(Menu),
m("h1", "Editing " + m.route.param("type"))
]
}
}
})
```

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.

### Path normalization

Parsed paths are always returned with all the duplicate parameters and extra slashes dropped, and they always start with a slash. These little differences often get in the way, and it makes routing and path handling a lot more complicated than it should be. Mithril internally normalizes paths for routing, but it does not expose the current, normalized route directly. (You could compute it via [`m.parsePathname(m.route.get()).path`](parsePathname.md).)

When parameters are deduplicated during matching, parameters in the query string are preferred over parameters in the path name, and parameters towards the end of the URL are preferred over parameters closer to the start of the URL.
16 changes: 13 additions & 3 deletions docs/request.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ m.request({

### Signature

`promise = m.request([url,] options)`
`promise = m.request(options)`

Argument | Type | Required | Description
------------------------- | --------------------------------- | -------- | ---
`url` | `String` | No | If present, it's equivalent to having the options `{method: "GET", url: url}`. Values passed to the `options` argument override options set via this shorthand.
`options` | `Object` | Yes | The request options to pass.
`options.method` | `String` | No | The HTTP method to use. This value should be one of the following: `GET`, `POST`, `PUT`, `PATCH`, `DELETE`, `HEAD` or `OPTIONS`. Defaults to `GET`.
`options.url` | `String` | Yes | The URL to send the request to. The URL may be either absolute or relative, and it may contain [interpolations](#dynamic-urls).
`options.url` | `String` | Yes | The [path name](paths.md) to send the request to, optionally interpolated with values from `options.data`.
`options.data` | `any` | No | The data to be interpolated into the URL and serialized into the querystring (for GET requests) or body (for other types of requests).
`options.async` | `Boolean` | No | Whether the request should be asynchronous. Defaults to `true`.
`options.user` | `String` | No | A username for HTTP authorization. Defaults to `undefined`.
Expand All @@ -62,6 +62,16 @@ Argument | Type | Required | Descr
`options.background` | `Boolean` | No | If `false`, redraws mounted components upon completion of the request. If `true`, it does not. Defaults to `false`.
**returns** | `Promise` | | A promise that resolves to the response data, after it has been piped through the `extract`, `deserialize` and `type` methods

`promise = m.request(url, options)`

Argument | Type | Required | Description
----------- | --------- | -------- | ---
`url` | `String` | Yes | The [path name](paths.md) to send the request to. `options.url` overrides this when present.
`options` | `Object` | No | The request options to pass.
**returns** | `Promise` | | A promise that resolves to the response data, after it has been piped through the `extract`, `deserialize` and `type` methods

This second form is mostly equivalent to `m.request(Object.assign({url: url}, options))`, just it does not depend on the ES6 global `Object.assign` internally.

[How to read signatures](signatures.md)

---
Expand Down
Loading

0 comments on commit c5c5698

Please sign in to comment.