Allowing dependency injection for global classes like XMLHttpRequest

[a1gemmel]

If this is a feature that would be of use to more people, I'd be happy to work on implementing it and drafting a PR with some guidance.

Motivation

In my use-case of mapbox-gl, I use client-side logic inside of tile requests in order to save network bandwidth and do tile generation on the fly that would otherwise require a server to proxy tile requests.

In particular, one use case is leveraging a binary coverage map format to avoid making requests for tiles that don't exist. The tile set I'm using does not have data of a homogenous ground resolution, and is sparse in many areas. The idea is that I can store one small file and query it on the client before sending tile requests out to network.

To do this I patch the send() function of the XMLHttpRequest prototype to inject my own pre-flight logic. This works fine, but it has the overhead of intercepting every request that the browser makes, and has the potential to introduce subtle bugs in other code that uses XMLHttpRequest.

In general, there are many global classes that MapBox uses that developers may want to override or mock up without impacting other code that uses those classes.

Design

Passing Dependencies

I propose two possible ways to do this:

1. Passing as part of the options object used by the Map constructor. That would look something like this:

var map = new mapboxgl.Map({
    container: 'map', // container id
    style: {
        ...
    },
    center: [-74.50, 40], // starting position
    maxZoom: 21,
    globals: {
        XMLHttpRequest: myXHR   
    }
});

2. Passing through a static method on mapboxgl.
   mapboxgl.DI("XMLHttpRequest", myXHR);

Using injected dependencies

The interface for using these classes inside MapBox should be made simple as possible.
References to window.THING could be replaced with mapboxgl.THING, which would return the injected implementation if it exists, or the window's original otherwise. The downside of that is that it does force developers to be aware of that convention.

Otherwise, the window object itself could be patched with proxies (which only moves the monkey-patching down a level :) ). It would even be possible to rewrite references to global objects during the build process, I'm concerned that would be seen as hacky though. Certainly there are multiple solutions for the interface, there's probably better ones than I've suggested here.

Implementation

The implementation of passing and storing dependencies is fairly straightforward, the uncertainty here is in what the API would look like. I would suggest using a Proxy object to store and delegate references to dependencies.

As for switching to using injected dependencies throughout the codebase, the implementation there will rely on what we want the interface to look like to the developer. It will take a bit of cleverness to make this transparent.

[asheemmamoowala]

@a1gemmel - Thank you for the detailed ticket ! We just added something similar in #5021. Would that solution work for your use case too ?
We are also working on a Custom Source API which should allow additional flexibility for defining or modifying sources.

[a1gemmel]

@asheemmamoowala thanks for the quick response!

I took a look at the diff and it does lay the groundwork for the specific functionality I need. On top of that diff I would need:

1. A transformOut for manipulating responses before handing them back to the callback in getArrayBuffer(), something like:

if (xhr.status >= 200 && xhr.status < 300 && xhr.response) {
      callback(null, {
        data: this.map._transformResponse(requestParameters, response),
        cacheControl: xhr.getResponseHeader('Cache-Control'),
        expires: xhr.getResponseHeader('Expires')
    });
}

2. A hook that takes in that same callback and the requestParameters and calls it with an arbitrary response. I haven't given much thought about the API for that but maybe it would return truthy if it called the callback (and so don't run an XHR request) and falsy otherwise. The motivation there is a way to short-circuit requests without going to network.

[a1gemmel]

Actually now that I think about it, the latter can implement the former.

[asheemmamoowala]

@a1gemmel - I think what you are looking for will be better solved by custom sources, when that is available. Overriding source behavior at the XMLHttpRequest is not the correct abstraction for this kind of behavior.

[a1gemmel]

@asheemmamoowala What is the timeline looking like for custom sources being released? I couldn't get a good idea by looking at that project page. Are there parts of that work that could be contributed independently or is it a fairly monolithic project?

[anandthakker]

@a1gemmel there's no concrete ETA on custom sources yet, but active design and development will be getting started in the next couple of weeks. Thanks for the offer to help out -- this may be possible in the later stages of the project, but I suspect it'll depend on some monolithic refactors being completed first.

[allthesignals]

Hi! Glad someone posted this. Any chance this is being implemented? I've run into a unique case where the global XMLHttpRequest object is being intercepted by some development tools (mock API stuff), and I need to be able to explicitly control how this is passed in. Sounds like custom sources are in the works...

[anandthakker]

Hi @allthesignals ! Custom sources are still in the plan, but they're still not yet under active development, unfortunately. Can you say a little more about your scenario/needs here, in case there might be some other workaround?

[allthesignals]

Hi @anandthakker, thanks for the response!

Sure! I'm using an app framework with an addon that acts as a fake in-browser backend API. It's for rapid prototyping purposes. It works by replacing the global window.XMLHttpRequest with an "interceptor" type object that mimics the API surface of XMLHttpRequest. It doesn't always do this perfectly, of course - there's a problem with how this response happens.

So, I'm hoping I can force dependency injection here:

mapbox-gl-js/src/util/ajax.js

Line 62 in 03680eb

const xhr: XMLHttpRequest = new window.XMLHttpRequest();

I'm looking at transformRequest which might help, but without some control I'm left with swapping in and out a global object, which is just begging for terrible, unpredictable race conditions.

Thank you!