Convert $.ajax calls to use fetch

[eritbh]

Related to #44

fetch is supported on all relevant browser versions and has a much nicer API than $.ajax does.

Because $.ajax passes three parameters to its callbacks, it's more difficult to have it play well with promises, which can only represent a single value (we get around this now by promising an object with the callback values as properties, but this isn't ideal). fetch, on the other hand, promises a single Response object.

Additionally, Response is (relatively) easily serializable to/from JSON - e.g. we can get the response from the background page, convert it to JSON, and then let TBUtils.sendRequest convert the JSON it received from the background page back into a Response object with its constructor. For example:

// background script
browser.runtime.onMessage.addListener(async request => {
    const response = await fetch('...');
    const headers = {};
    for (const [k, v] of response.headers) {
        headers[k] = v;
    }
    return [response.text(), {
        status: response.status,
        statusText: response.statusText,
        headers,
    }];
});
// content script
const response = await browser.runtime.sendMessage({...}).then(responseData => new Response(...responseData));
doSomethingWith(response.json());

[creesch]

Just as a headsup, fetch seems simpler but also needs more scaffolding to make it actually work properly in regards to error handling. See this.

I also remember reading it doesn't work as nicely with cookies and sessisons as xhr does.

Anyway, if we can replace the jquery ajax calls with it then it is absolutely fine.

Alternatively we could use axios which is promise based and a solid framework.

[eritbh]

That article seems to make things a bit more complicated than it has to in regards to error handling. Since all our requests are happening from one bit of code, it's easy enough to add .then(response => response.ok ? response : Promise.reject(response)) to the helper, and then we can catch non-OK responses along with incomplete requests just like normal.

Cookie support seems easy enough too, we just need to add the credentials parameter, which again, since we're using helper functions for all our requests, can easily be added for all requests. https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch#Parameters

All good things to keep in mind when we implement it though. This change will definitely need a fair amount of testing before it's good to go.

[creesch]

Stickied this one as we are currently running into some issues it seems with ajax jquery, specifically redirects when reddit thinks you should log in. ajax can't pick up on them and then we get served an html pages.

So it might be beneficial to explore this a bit further soonish.

After recently having played around with fetch it seems indeed that the scaffolding surrounding it is doable.

For example this is what I did in another project modified to work on reddit.

function handleEndpoint ({endpoint, method}) {
    return new Promise((resolve, reject) => {
        // If we don't have a method we can't continue
        if (!method) {
            method = 'GET'
        }

        // Set up the initial request params.
        const requestInit = {
            method,
			credentials: 'include',
        };

        let requestURL = `https://old.reddit.com${endpoint}`;

        const apiRequest = new Request(requestURL, requestInit);

        fetch(apiRequest)
            .then(response => {
                if (response.ok) {
                    return response;
                }
                return reject({
                    status: response.status,
                    statusText: response.statusText,

                });
            })
            .then(response => response.json())
            .then(response => {
                resolve(response);
            });
    });
}

Called through

    handleEndpoint ({
        endpoint: '/subreddits/mine/moderator.json', 
    })
        .then(response => {
            console.log(response)
        }).catch(error => {
            console.error(error);
        });

Which as you pointed already out can likely be simplified (and isn't complete either) :P Just pasting it in here as reference.

[creesch]

Oh what I wanted to actually type is that we need to make sure we can handle redirects with fetch properly 😁

I also tested the above code (and modified it from the initial code I pasted) and it seems to work fairly well in the background.

edit:

https://developer.mozilla.org/en-US/docs/Web/API/Response/redirected

Seems like if we add redirect: "error" that it will throw an error.

[eritbh]

I'm gonna look into implementing this soonish. Do we generally want to disallow redirects or follow them?

[creesch]

Error on redirect I believe.

[creesch]

As I got reminded by the other jquery related issue we tackled the other day, any progress here? There is no rush as using jquery for now is fine.

[eritbh]

Still on my radar for when I'm done with (or get bored of) #245