mithril.js icon indicating copy to clipboard operation
mithril.js copied to clipboard

Mithril's default `extract` should be exposed so users can fall back on it

Open dead-claudia opened this issue 4 years ago • 2 comments

Mithril version: 2.0.4

Browser and OS: All

Project:

Is this something you're interested in implementing yourself? Yes

Description

Mithril has some pretty involved logic for extract, much of which is far from obvious (and involves years of bugs), and you can't use both extract and deserialize together.

I suggest we package that up into a single m.request.extract(xhr, opts) such that we can default extract to m.request.extract.

Why

It's a fairly common use case for people to want to only extract some requests specially, not all. Also, some may want to use Mithril's normal extract functionality, but also throw errors based on XHR headers or other out-of-band info on the XHR object itself prior to unwrapping.

I've seen this come up many times on Gitter.

Possible Implementation

It's a fairly straightforward refactor pulling this code out into a separate function that simply returns its result directly and updating m.request with something to this effect:

// At the top of the `request` callback
var extract = args.extract || m.request.extract

// ...

xhr.onreadystatechange = function (ev) {
	// Don't throw errors on xhr.abort().
	if (aborted) return

	if (ev.target.readyState === 4) {
		try {
			resolve(
				typeof args.extract === "function"
					? args.extract(ev.target, args)
					: m.request.extract(ev.target, args)
			)
		} catch (e) {
			reject(e)
		}
	}
}

Open Questions

dead-claudia avatar Mar 24 '20 02:03 dead-claudia

I can't even tell based on the documentation on how to use extract for Mithriljs 2.0 Maybe a separate issue.

omenking avatar Apr 08 '20 01:04 omenking

@omenking Definitely file a separate issue. This is about a feature proposal, not a bug.

dead-claudia avatar Apr 08 '20 19:04 dead-claudia