MithrilJS / docs

Source code for Mithril's documentation site
https://mithril.js.org
MIT License
1 stars 4 forks source link

Our docs' function signatures can get unreadable in a hurry. #21

Open dead-claudia opened 5 years ago

dead-claudia commented 5 years ago

Currently, we use a relatively unusual signature mechanism to document how our functions are called. It's nice and concise for a few simple cases, but it has a habit of getting in the way of readability and accuracy in the more complicated scenarios:

I've only covered about a third of the signatures in links above, and only individual bits of them in the bulleted list. But if you actually look at how it renders, I'd estimate about half of them have issues generated from the signature format itself.


What I'm really proposing here is that we should change our signature format to be much more readable. I'm thinking of something like this, using m() as an example:

### Signature

```js
vnode = m(selector, attrs = {}, ...children)
vnode = m(selector, attrs = {}, [...children])

Returns a hyperscript vnode for Mithril to later render.

How to read signatures


Rendered:

> ### Signature
>
> ```js
> vnode = m(selector, attrs = {}, ...children)
> vnode = m(selector, attrs = {}, [...children])
> ```
>
> - `selector` - A component or [simple CSS selector](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors#Simple_selectors) string.
> - `attrs` - A key/value map of HTML attributes, element properties, and/or [lifecycle methods](https://mithril.js.org/hyperscript.html#lifecycle-methods).
> - `children` - A list of vnode children, including strings, numbers, vnodes, or arrays of vnode children. `null`, `undefined`, and `false` represent holes.
>
> Returns a hyperscript vnode for Mithril to later [render](https://mithril.js.org/render.html).
>
> [*How to read signatures*](https://mithril.js.org/signatures.md)

For comparison, [here's the existing docs](https://mithril.js.org/archive/v2.0.0-rc.4):

`````markdown
`vnode = m(selector, attrs, children)`

Argument     | Type                                       | Required | Description
------------ | ------------------------------------------ | -------- | ---
`selector`   | `String|Object`                            | Yes      | A CSS selector or a [component](components.md)
`attrs`      | `Object`                                   | No       | HTML attributes or element properties
`children`   | `Array<Vnode>|String|Number|Boolean`       | No       | Child [vnodes](vnodes.md#structure). Can be written as [splat arguments](signatures.md#splats)
**returns**  | `Vnode`                                    |          | A [vnode](vnodes.md#structure)

[How to read signatures](signatures.md)

Rendered:

vnode = m(selector, attrs, children)

Argument Type Required Description
selector String\|Object Yes A CSS selector or a component
attrs Object No HTML attributes or element properties
children Array<Vnode>\|String\|Number\|Boolean No Child vnodes. Can be written as splat arguments
returns Vnode A vnode

How to read signatures

I'm open to other ideas, like using TypeScript types, as long as they're an improvement over what we currently have.

In case you're curious how I'd change m.request, here's how I'd do that: `````markdown ### Signature ```js promise = m.request(url, options = {}) promise = m.request({url, ...options}) promise.then( function(result) { ... }, function(error) { ... } ) ``` - `url` - The URL to send the request to. It may be either absolute or relative, and it may contain [interpolations](#dynamic-urls). - Options: - `options.method = "GET"` - The [HTTP method](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) to use. - `options.data = undefined` - 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 = true` - Whether the request should be fired asynchronously. - `options.user = null` - The username for [HTTP basic authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication). - `options.password = null` - The password for [HTTP basic authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication). - `options.withCredentials = false` - Whether to send cookies to 3rd party domains. - `options.timeout = undefined` - The amount of milliseconds a request can take before automatically being [terminated](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout). - `options.responseType = undefined` - The expected [type](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType) of the response. - `options.config(xhr) -> newXhr = function(x) { return x }` - Exposes the underlying XMLHttpRequest object for low-level configuration. - `options.headers = {}` - Headers to append to the request before sending it, applied right before `options.config`. - `new options.type(response) -> result = ...` - A constructor to be applied to each object in the response if it's an array or the response itself otherwise. By default, the response is used directly. - `options.serialize(options.data) -> string = ...` - Serialize the data into a string. By default, this returns `options.data` if it's an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData) or `JSON.stringify(options.data)` otherwise. - `options.deserialize(responseText) -> response = ...` - Deserialize `xhr.responseText` into the response data. By default, this returns `null` for empty responses and `JSON.parse(responseText)` otherwise. - `options.extract(xhr, options) -> response = ...` - A hook to specify how the [`XMLHttpRequest`](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest) response should be read, useful for processing response data, reading headers, and cookies. By default, this returns `options.deserialize(xhr.responseText)` for requests that were successful (status either between 200 and 299 or 304), throws an error for requests that were not. - `options.useBody = (options.method !== "GET")` - Force the use of the HTTP body section for `data` in `GET` requests when set to `true`, or the use of querystring for other HTTP methods when set to `false`. - `options.background = false` - If `false`, redraws mounted components upon completion of the request. If `true`, it does not. Returns a promise to the response, after it's been piped through `options.extract`, `options.deserialize`, and `options.type` as appropriate. Promise rejections outside the above hooks model request errors, and have a few special properties: - `error.message` - The raw response text - `error.code` - The server status code - `error.response` - The parsed response If a custom `options.extract` is provided, `deserialize` will be skipped and the value returned from the extract callback will be left as-is when the promise resolves. Furthermore, exceptions are *not* thrown when the server response status code indicates an error. Instead, it's expected that you determine in the callback whether the error was in fact a server error or a normal response. [How to read signatures](https://mithril.js.org/signatures.md) ````` Rendered: > ### Signature > > ```js > promise = m.request(url, options = {}) > promise = m.request({url, ...options}) > > promise.then( > function(result) { ... }, > function(error) { ... } > ) > ``` > > - `url` - The URL to send the request to. It may be either absolute or relative, and it may contain [interpolations](#dynamic-urls). > - Options: > - `options.method = "GET"` - The [HTTP method](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) to use. > - `options.data = undefined` - 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 = true` - Whether the request should be fired asynchronously. > - `options.user = null` - The username for [HTTP basic authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication). > - `options.password = null` - The password for [HTTP basic authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication). > - `options.withCredentials = false` - Whether to send cookies to 3rd party domains. > - `options.timeout = undefined` - The amount of milliseconds a request can take before automatically being [terminated](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout). > - `options.responseType = undefined` - The expected [type](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType) of the response. > - `options.config(xhr) -> newXhr = function(x) { return x }` - Exposes the underlying XMLHttpRequest object for low-level configuration. > - `options.headers = {}` - Headers to append to the request before sending it, applied right before `options.config`. > - `new options.type(response) -> result = ...` - A constructor to be applied to each object in the response if it's an array or the response itself otherwise. By default, the response is used directly. > - `options.serialize(options.data) -> string = ...` - Serialize the data into a string. By default, this returns `options.data` if it's an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData) or `JSON.stringify(options.data)` otherwise. > - `options.deserialize(responseText) -> response = ...` - Deserialize `xhr.responseText` into the response data. By default, this returns `null` for empty responses and `JSON.parse(responseText)` otherwise. > - `options.extract(xhr, options) -> response = ...` - A hook to specify how the [`XMLHttpRequest`](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest) response should be read, useful for processing response data, reading headers, and cookies. By default, this returns `options.deserialize(xhr.responseText)` for requests that were successful (status either between 200 and 299 or 304), throws an error for requests that were not. > - `options.useBody = (options.method !== "GET")` - Force the use of the HTTP body section for `data` in `GET` requests when set to `true`, or the use of querystring for other HTTP methods when set to `false`. > - `options.background = false` - If `false`, redraws mounted components upon completion of the request. If `true`, it does not. > > Returns a promise to the response, after it's been piped through `options.extract`, `options.deserialize`, and `options.type` as appropriate. Promise rejections outside the above hooks model request errors, and have a few special properties: > > - `error.message` - The raw response text > - `error.code` - The server status code > - `error.response` - The parsed response > > If a custom `options.extract` is provided, `deserialize` will be skipped and the value returned from the extract callback will be left as-is when the promise resolves. Furthermore, exceptions are *not* thrown when the server response status code indicates an error. Instead, it's expected that you determine in the callback whether the error was in fact a server error or a normal response. > > [How to read signatures](https://mithril.js.org/signatures.md) For comparison, [here's the original docs](https://mithril.js.org/archive/v2.0.0-rc.4/request.html#signature): `````markdown ### Signature `promise = m.request([url,] 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.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.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`. `options.password` | `String` | No | A password for HTTP authorization. Defaults to `undefined`. This option is provided for `XMLHttpRequest` compatibility, but you should avoid using it because it sends the password in plain text over the network. `options.withCredentials` | `Boolean` | No | Whether to send cookies to 3rd party domains. Defaults to `false` `options.timeout` | `Number` | No | The amount of milliseconds a request can take before automatically being [terminated](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout). Defaults to `undefined`. `options.responseType` | `String` | No | The expected [type](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType) of the response. Defaults to `undefined`. `options.config` | `xhr = Function(xhr)` | No | Exposes the underlying XMLHttpRequest object for low-level configuration. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function). `options.headers` | `Object` | No | Headers to append to the request before sending it (applied right before `options.config`). `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.serialize` | `string = Function(any)` | No | A serialization method to be applied to `data`. Defaults to `JSON.stringify`, or if `options.data` is an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData), defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function) (i.e. `function(value) {return value}`). `options.deserialize` | `any = Function(string)` | No | A deserialization method to be applied to the `xhr.responseText`. Defaults to a small wrapper around `JSON.parse` that returns `null` for empty responses. If `extract` is defined, `deserialize` will be skipped. `options.extract` | `any = Function(xhr, options)` | No | A hook to specify how the XMLHttpRequest response should be read. Useful for processing response data, reading headers and cookies. By default this is a function that returns `xhr.responseText`, which is in turn passed to `deserialize`. If a custom `extract` callback is provided, the `xhr` parameter is the XMLHttpRequest instance used for the request, and `options` is the object that was passed to the `m.request` call. Additionally, `deserialize` will be skipped and the value returned from the extract callback will be left as-is when the promise resolves. Furthermore, when an extract callback is provided, exceptions are *not* thrown when the server response status code indicates an error. `options.useBody` | `Boolean` | No | Force the use of the HTTP body section for `data` in `GET` requests when set to `true`, or the use of querystring for other HTTP methods when set to `false`. Defaults to `false` for `GET` requests and `true` for other methods. `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 [How to read signatures](signatures.md) ````` Rendered: > ### Signature > > `promise = m.request([url,] 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.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](https://mithril.js.org/archive/v2.0.0-rc.4/request.html#dynamic-urls). > `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`. > `options.password` | `String` | No | A password for HTTP authorization. Defaults to `undefined`. This option is provided for `XMLHttpRequest` compatibility, but you should avoid using it because it sends the password in plain text over the network. > `options.withCredentials` | `Boolean` | No | Whether to send cookies to 3rd party domains. Defaults to `false` > `options.timeout` | `Number` | No | The amount of milliseconds a request can take before automatically being [terminated](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout). Defaults to `undefined`. > `options.responseType` | `String` | No | The expected [type](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType) of the response. Defaults to `undefined`. > `options.config` | `xhr = Function(xhr)` | No | Exposes the underlying XMLHttpRequest object for low-level configuration. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function). > `options.headers` | `Object` | No | Headers to append to the request before sending it (applied right before `options.config`). > `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.serialize` | `string = Function(any)` | No | A serialization method to be applied to `data`. Defaults to `JSON.stringify`, or if `options.data` is an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData), defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function) (i.e. `function(value) {return value}`). > `options.deserialize` | `any = Function(string)` | No | A deserialization method to be applied to the `xhr.responseText`. Defaults to a small wrapper around `JSON.parse` that returns `null` for empty responses. If `extract` is defined, `deserialize` will be skipped. > `options.extract` | `any = Function(xhr, options)` | No | A hook to specify how the XMLHttpRequest response should be read. Useful for processing response data, reading headers and cookies. By default this is a function that returns `xhr.responseText`, which is in turn passed to `deserialize`. If a custom `extract` callback is provided, the `xhr` parameter is the XMLHttpRequest instance used for the request, and `options` is the object that was passed to the `m.request` call. > Additionally, `deserialize` will be skipped and the value returned from the extract callback will be left as-is when the promise resolves. Furthermore, when an extract callback is provided, exceptions are *not* thrown when the server response status code indicates an error. > `options.useBody` | `Boolean` | No | Force the use of the HTTP body section for `data` in `GET` requests when set to `true`, or the use of querystring for other HTTP methods when set to `false`. Defaults to `false` for `GET` requests and `true` for other methods. > `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 > > [How to read signatures](https://mithril.js.org/archive/v2.0.0-rc.4/signatures.md)
dead-claudia commented 5 years ago

Basically, it's moving crap from tables to code blocks with associated prose so it's easier to follow. My proposal could be summarized as this:

spacejack commented 5 years ago

Agreed. I've edited the request docs once or twice and it's not fun in its current form. :)

delventhalz commented 5 years ago

Hey there. Long time listener, first time caller. I'm thinking about picking this up just to get my feet wet. How would you feel about modifying the original suggestion slightly to more closely monkey how MDN formats their docs? I'm a big fan of stealing from the best. Something like this:

### Syntax

m(selector[, attrs[, ...children]])


#### Parameters

- `selector`

  A component or [simple CSS selector](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors#Simple_selectors) string.

- `attrs` | _optional_

  A key/value map of HTML attributes, element properties, and/or [lifecycle methods](https://mithril.js.org/hyperscript.html#lifecycle-methods).

- `children` | _optional_

  A list of vnode children, including strings, numbers, vnodes, or arrays of vnode children. `null`, `undefined`, and `false` represent holes.

#### Return value

A hyperscript vnode for Mithril to later [render](https://mithril.js.org/render.html).

[*How to read signatures*](https://mithril.js.org/signatures.md)

And rendered:

Syntax

m(selector[, attrs[, ...children]])

Parameters

  • selector

    A component or simple CSS selector string.

  • attrs | optional

    A key/value map of HTML attributes, element properties, and/or lifecycle methods.

  • children | optional

    A list of vnode children, including strings, numbers, vnodes, or arrays of vnode children. null, undefined, and false represent holes.

Return value

A hyperscript vnode for Mithril to later render.

How to read signatures

I think this could offer a few advantages:

orbitbot commented 5 years ago

IMO the original proposal works for most stuff, but m.request is just messy no matter what if everything is explained at once. Might be easier to read with the MDN syntax perhaps. Otherwise, I guess people might be somewhat familiar with typescript annotations by now, but I don't really have enough experience with those to say if that is some kind of format that might be useful for this level of documentation (?).

The MDN style for XHR is of a similar length to m.request and at least a bit better than the table and wall of bullet points: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest

dead-claudia commented 5 years ago

@orbitbot We could hide the bullet points by just wrapping the signature in a <div class="signature"> and a single line of CSS.

As for TS-like vs MDN, I'm neutral. I personally prefer to avoid explicit type names, sticking a little closer to MDN with a few TS/ES things like optional? and ...rest arguments, but I'm not beholden to that. I've found variable names to be sufficient to convey types, providing the flexibility you'd get out of TS without necessarily using a TS type. I would, however, be against going as far as to use TS generics, interface declarations, and the like.

And MDN's use of method(arg[, optional]) in its JS docs is based on the ES spec which uses a similar idiom. The DOM uses WebIDL, obviously, and MDN generally prefers that for its HTML/DOM API documentation. I don't see the ES spec style very often elsewhere (I normally see optional?, optional = value, or prose - jQuery is an exception), and I only usually see the WebIDL style in older libraries and utilities like Uglify and a few other things. BTW, our current documentation style is very jQuery-like, so we aren't exactly on an island. The main difference is they use rows and line breaks, where we use a table.

Something aligned with what jQuery does would look like this:

`m(selector [, attrs] [, child1] [, childN])`

*Returns: a [vnode](vnodes.md#structure)`*

- `selector`<br>
  Type: String or Object<br>
  A CSS selector or a [component](components.md)

- `attrs`<br>
  Type: Object<br>
  HTML attributes or element properties

- `child1`<br>
  Type: String, Number, Boolean, a vnode, or an array of children<br>
  A child [vnode](vnodes.md#structure).

- `childN`<br>
  Type: String, Number, Boolean, a vnode, or an array of children<br>
  Additional child [vnodes](vnodes.md#structure).

[How to read signatures](signatures.md)

Rendered:

m(selector [, attrs] [, child1] [, childN])

Returns: a vnode`

  • selector
    Type: String or Object
    A CSS selector or a component

  • attrs
    Type: Object
    HTML attributes or element properties

  • child1
    Type: String, Number, Boolean, a vnode, or an array of children
    A child vnode.

  • childN
    Type: String, Number, Boolean, a vnode, or an array of children
    Additional child vnodes.

How to read signatures

orbitbot commented 5 years ago

To be clear, my issue with the documentation in the case of something like the amount of options that m.request has is that it isn't readable, not that there are bullet points there. Since the amount of text is so large, there needs to be some kind of separation between each bullet point for it to be even remotely legible, so in that case the MDN style is definitely better.

Without proper spacing for the "wall of bullet points", it's essentially like reading a long text without typography, where indentation and line breaks between paragraphs have been removed.

I don't really have that much issue with MDN or jQuery style, other than that with a lot of options it starts to consume so much vertical space that the reader might lose the context. Not sure how relevant the subheadings are in MDN's XHR docs, but I don't really keep track of what section I'm reading, and this is with zooming out (Chrome's site zooming) a bit so I still have more vertical space visible.

delventhalz commented 5 years ago

If the parameter description is in its own paragraph, it gives you room to inline a type annotation if you wanted to include it:

  • selector: String|Object A CSS selector or a component

Though it might be clearer with a JSDoc style annotation:

  • selector {String|Object} A CSS selector or a component

A few ways to include an optional tag with an inline type too:

  • attrs?: Object A key/value map of HTML attributes, element properties, and/or lifecycle methods.

  • attrs {Object} (optional) A key/value map of HTML attributes, element properties, and/or lifecycle methods.

  • attrs - {Object} - optional A key/value map of HTML attributes, element properties, and/or lifecycle methods.

As for the function signature itself, I think the use of brackets to denote optional parameters is really common and worth replicating. Not really particular on whether those are nested (MDN), sequential (jQuery) or whatever.

I don't think trying to make the signature more JS-like (as in the original suggestion) is a great approach. It's not really valid JS as written anyway. So I would personally rather have some good example code which is 100% valid, best-practice, JS, and then something distinct but clear which documents the signature.