Define high-level API

[tgvashworth]

• How do you add plugins?
• Can they be added (or removed?) at runtime?
• What does 'hooking' look like?

[tgvashworth]

A quick sketch...

import { FetchEngine, FetchCancel, FetchContinue } from 'fetch-engine';

// Basic no-op fetch wrapper
let fetch = new FetchEngine();

// RateLimit
class RateLimitPlugin {
  preFetch(request) {
    if (/* is rate limited */) {
      return new FetchCancel();
    }
    return new FetchContinue();
  }
}

// Timeouts
class TimeoutPlugin {
  constructor(time=1000) {
    this.time = time;
  }

  onFetch(promise) {
    return new Promise((resolve, reject) => {
      setTimeout(() => reject(new Error()), this.time);
      promise.then(resolve, reject);
    });
  }
}

let fetch = new FetchEngine({
  plugins: [
    new RateLimitPlugin(),
    new TimeoutPlugin(1000)
  ]
});

[kkemple]

So basically { preFetch, onFetch, postFetch } are the methods you can declare in a plugin... then you just add plugins when you instantiate

I guess this would be some sort of async reduce that calls the methods in order they are added (getting ahead of myself here, but just curious if you thought about that yet)?

It makes sense to me, what do you see FetchContinue and FetchCancel as? Kind of like Promise.resolve() and Promise.reject(), but handling the cancelation of requests and de-duping?

PS. This is great, I haven't seen a really well thought out solution to handling more complex data fetching needs and it looks like you plan to build what I have been looking for! :tada: :confetti_ball:

[tgvashworth]

So basically { preFetch, onFetch, postFetch } are the methods you can declare in a plugin... then you just add plugins when you instantiate

Exactly!

I guess this would be some sort of async reduce that calls the methods in order they are added

Something like that is what I was thinking, although a reduce is tricky if you want to cancel early as you can't break out of it (afaik?)

It makes sense to me, what do you see FetchContinue and FetchCancel as? Kind of like Promise.resolve() and Promise.reject(), but handling the cancelation of requests and de-duping?

Yep – they're a kind of signal type. I should expand on that a bit more.

Thanks for taking a look! I really appreciate it.

[jackfranklin]

@phuu this idea seems really cool - if you'd like some collaborators LMK.

[tgvashworth]

@jackfranklin Definitely — what do you think of the API?

One thing I'm thinking is that there are going to be cases where you want a common set of behaviour for all network requests, but some extra for some subset. I hate subclassing (fortunately not obvious to subclass the FetchEngine with this API) but it would be nice to share. I wonder if there's a good example of something like that in another project.

Might even be possible like this:

const fetch = new FetchEngine({
  plugins: [ ... ]
});

const authedFetch = new FetchEngine({
  plugins: fetch.plugins.concat([ ... ])
});

[jackfranklin]

It's unclear exactly how FetchContinue and FetchCancel will work - I'd be interested in the semantics of those.

Otherwise the API looks nice - this seems like a nice follow up to https://github.com/jackfranklin/fetch-factory and the idea of proving a client out of the box that plays nicely is really cool.

I agree with your idea for adding plugins but I'd make the API:

const existingFetch = ...;

const authedFetch = new FetchEngine({
  plugins: [ existingFetch.plugins, new FooPlugin() ]
});

I'm nit picking now though ;)

[kkemple]

@phuu

One thing I'm thinking is that there are going to be cases where you want a common set of behaviour for all network requests, but some extra for some subset.

Could it not be up to the plugin to decide if it should act on a request? This almost feels like a situation where you would investigate meta data or the url of the request and filter work streams like that, this allows for a single instance instead of managing multiple instances (although not really that bad), i.e.

class GitHubAuthPlugin {
  preFetch(request) {
    if (/* url matches github api */) {
      request.headers['authorization'] = ...
    }
    return new FetchContinue();
  }
}

const fetch = new FetchEngine({
  plugins: [ new GitHubAuthPlugin() ]
});

[joews]

@phuu this looks excellent!

My 2p on re-using plugins without inheritance: I prefer concat to @jackfranklin's suggestion because it avoids the complexity of nested someFetch.plugins arrays.

It reads nicely with a spread:

const fetch = new FetchEngine({
  plugins: [...existingFetch.plugins, new FooPlugin()]
});

[jackfranklin]

@jwhitfieldseed forgot about spread, big :+1: to that

[tgvashworth]

@jackfranklin @jwhitfieldseed :+1: to spread, although .plugins adds to the interface of fetch which is potentially icky.

@kkemple That's true, but it's harder to do that for cases like "retry these fetches on 503, but not these" or to configure your back off on specific endpoints. However, this does raise the question: could the plugins be externally configurable to only apply to certain endpoints.

For example, off the top of my head:

const LoginBackoffPlugin = new RequestFilteredPlugin({
  path: '/login',
  plugins: [
    new BackoffPlugin({
      strategy: t => Math.pow(2, t)
    })
  ]
});

However, I'm quite sure there's a better abstraction for this. Now it's just a case of finding it. I'll create a new issue for the plugin composition.

[kkemple]

:+1:

[passy]

I like the idea of FetchCancel and FetchContinue, but they seem to me like singletons; something I'd model as a sum type in other languages, e.g. data FetchProgress = FetchContinue | FetchCancel. Not that it should matter, but using new here means a new allocation that seems to be unneeded if this is just about having an identifier that is compared against at a later stage.

[tgvashworth]

@passy Yeah, great point. Same with None and Some in #3.

[tgvashworth]

I done some thinkin', and I reckons the following will work. It's based on ideas from React, Babel ('preset') and others so kudos to them for being fab.

There'd be two main constructors that come with fetch-engine: FetchEngine and FetchPreset. What they'd do is documented below:

FetchEngine

const fetch = new FetchEngine({
  plugins: [ ... ]
});

• on creation, compose each plugin method into one method
• on fetch(...), run methods in order:
  • shouldFetch
  • if false, bail with CancelledError
  • getRequest
  • produces a (Promise for a) Request
  • willFetch
  • side-effects, ignore return value
  • (internal-only) fetch
  • make the Request with an inner fetch implementation
  • fetch
  • allow plugins to cancel
  • getResponse
  • produce a (Promise for a) Response
  • didFetch
  • side-effects only, ignore return value

FetchPreset

const preset = new FetchPreset({
  filters: [ ... ],
  plugins: [ ... ]
});

• compose filter + plugin methods into functions, then act like a plugin
• gates each method based on filter responses

Plugins

The plugin API would be as below. They're just simple objects with methods. All are optional.

shouldFetch

Passed the current Request object.

Allows a plugin to prevent a request from being made.

Return a (Promise for a) Boolean. If false, the request is Cancelled.

getRequest

Passed the current Request object.

Allows plugins to add data to Request, or produce an entirely new Request.

Should return a (Promise for a) Request.

willFetch

Passed the current Request object.

Allows a plugin to react to a request being made, but not affect it.

Return value would be ignored.

fetch

Passed an object of the form { promise, cancel }.

Allows plugins to react to the completion of the fetch but not affect it, or cancel the request.

Return value is ignored.

getResponse

Passed the current Response object.

Allows plugins to add data to Response, or produce an entirely new Response.

Should return a (Promise for a) Response.

didFetch

Passed the current Response object.

Allows a plugin to react to a response arriving being made, but not affect it.

Return value would be ignored.

Filters

The filter API would be as below. Filters are used to gate the application of plugins within a FetchPreset. They're just simple objects with methods. All are optional.

testRequest

Passed the current Request.

Run before shouldFetch, getRequest, willFetch and fetch, which will not be applied if testRequest resolves to false.

Should return a (Promise for a) Boolean.

testResponse

Passed the current Response.

Run before getResponse and didFetch, which will not be applied if testResponse resolves to false.

Should return a (Promise for a) Boolean.

Example

Here's an example without an implementation, just in case this is super unclear...

import { FetchEngine, FetchPreset } from 'fetch-engine';

class PathPrefixFilter {
  constructor(prefix) {
    this.prefix = prefix;
  }
  testRequest(request) {
    const url = new URL(request.url);
    return url.pathname.startsWith(prefix);
  }
}

class CORSAuthPlugin {
  getRequest(request) {
    return new Request(request, {
      mode: 'cors',
      credentials: 'include',
      headers: Object.assign(request.headers, {
        'X-Csrf-Token': getCsrfToken()
      })
    });
  }
}

class RateLimitPlugin {
  shouldFetch(request) {
    return !isRateLimited(request);
  }
}

class TimeoutPlugin {
  constructor(time=1000) {
    this.time = time;
  }
  fetch({ cancel }) {
    setTimeout(cancel, this.time);
  }
}

class MetricsPlugin {
  willFetch(request) {
    trackRequest(request);
  }
  didFetch(response) {
    trackResponse(response);
  }
}

let fetch = new FetchEngine({
  plugins: [
    new TimeoutPlugin(5000),
    new CORSAuthPlugin(),
    new FetchPreset({
      filters: [ new PathPrefixFilter('/1.1/') ],
      plugins: [ new RateLimitPlugin() ]
    }),
    new MetricsPlugin()
  ]
});

[jackfranklin]

@phuu that looks really nice, big :+1: from me.

[kkemple]

i like the request lifecycle you've outlined, it feels very natural, so :+1: to that

question about FetchPreset, it allows for finer grained control over what causes the plugins to run, via the filters. So if the filter returns false or the promise rejects then the plugins for that FetchPreset will not be run against that request/response, is that correct?

[passy]

@phuu Awesome work! A question about the format here as this is now getting closer to a "real spec", what do you think about adding some types to this; either through TypeScript, Flow or IDL? A lot of "for side-effects" would be clear by having a void or unit function.

Another question, actually related to types, when you say "Should return a (Promise for a) Boolean." do you mean that the method could return both a "raw" and a "promisefied" boolean value? I'm not a big fan of that kind of polymorphism as it leads to ugly instanceof chains and makes it harder to write wrappers in other languages. To me having to write Promise.resolve(_) doesn't seem like a lot of additional work for a cleaner interface. But maybe that's already entering the bikeshedding territory ...

How do errors affect the life cycle? It's not entirely clear to me which methods would be called if a request was aborted at the various stages. Error responses seem clear, but what happens if something within the chain would throw an error? If we have a timeout and the promise is rejected, will didFetch ever get called?

[tgvashworth]

@kkemple About FetchPreset, you have it exactly right. I also played with nested FetchEngine instances by making them support filters but it didn't quite work out.

@passy Absolutely I'd like to type this. Got a preferred option? And thanks for the question about errors, they're important to spec out. My intuition is that methods will only fire on a successful completion of the previous, meaning no JS-level errors occurred. Any negative implication there?

On the (Promise for a) X question, @passy and I discussed this face to face, but adding it here for posterity...

I do mean both, as JS devs are used to the automatic-flatMap that the Promise API affords (then, resolve etc all support Promises and raw values). In that sense, I'd like to keep things familiar.

[passy]

@phuu I don't have a real preference when it comes to type defs, but TypeScript definition files seem to be quite common these days and it's fairly easy to find similar examples.

[tgvashworth]

Thinking a bit more, the FetchEngine constructor returning a function is bit odd. Perhaps it can just be a function...

import { fetchEngine } from 'fetch-engine';
const fetch = fetchEngine({
  plugins: [ ... ]
});

[tgvashworth]

FetchGroup is also nicer than FetchPreset.

FetchGroup actually makes for a potentially nice pattern:

var fetch = fetchEngine(new FetchGroup({
  plugins: [
    new FetchGroup({
      filters: [ ... ],
      plugins: [ ... ]
    })
  ]
});

It could be shorthanded (so if you pass an object, the FetchGroup is created for you), but essentially things nest rather nicely.

[kkemple]

:+1: FetchGroup > FetchPreset

[tgvashworth]

Ok, closing this as the API is coming together in the ts files. Thanks for your help everyone.