Documentation for xhrOptions.extract

[hankeypancake]

I noticed that the spec for m.requests option extract says that the default function is

function(xhr, xhrOptions){ return xhr.responseText };

This is not the case? from the source (https://github.com/lhorie/mithril.js/blob/master/mithril.js#L1117-L1119):

var extract = isJSONP ? function(jsonp) {return jsonp.responseText} : xhrOptions.extract || function(xhr) {
    return xhr.responseText.length === 0 && deserialize === JSON.parse ? null : xhr.responseText
};

since the return value from extract always gets passed to deserialize, the deserialize function must be able to parse the return value. The default value for the deserialize function is JSON.Parse, so if we just add a extract function without changing the deserialize function to handle the return value, like in the example in the documentation, m.request will fail because JSON.parse fails (as long as it's not a valid JSON-string)

// from https://lhorie.github.io/mithril/mithril.request.html#extracting-metadata-from-the-response

var extract = function(xhr, xhrOptions) {
    if (xhrOptions.method == "HEAD") return xhr.getResponseHeader("x-item-count")
    else return xhr.responseText
}

m.request({method: "POST", url: "/foo", extract: extract});

(this makes no sense anyways, why would we look for method == "HEAD" when we instantly after send a POST. Neither POST nor HEAD will work though, if we don't modify deserialize)

so, to my question: Should extract/deserialize be implemented like this, or should the documentation just be changed so that it actually tells you what happens?

[dead-claudia]

The documentation is mostly correct, but it glosses over a special case where the server responds with an empty string, and the default deserialize, JSON.parse, is used.

In this case, if you try JSON.parse(""), that will throw a SyntaxError, but throwing an error here doesn't make sense, because even though it's technically invalid JSON, an empty response with a status code of 200 is still reasonable from a poorly written server. It's not something like {"foo":, which should never happen.

IMHO the check is in the wrong place, but it's otherwise unobservable beyond that edge case unless you add a getter or setter property with that name or use a Proxy (which breaks most API contracts, anyways).

[hankeypancake]

Maybe it's mostly the example that's incorrect/incomplete.

1. We don't send a HEAD request, so we will never extract any metadata from the headers.
2. If we sent a HEAD request, the example would return a string, which wouldn't work with the default deserializer.

[dead-claudia]

For what it's worth, a small docs fix to note the corner case with an empty 200 response would be nice. :smile:

[dead-claudia]

Closing, since the 0.2.x docs are no longer live. It's recommended you migrate to 1.x, which has the recommended fix.