Skip to content

Commit

Permalink
Merge pull request #13 from quran/dev
Browse files Browse the repository at this point in the history
fix: don't mutate mergeApiOptions params so that fetchFn works
  • Loading branch information
ahmedriad1 authored Apr 9, 2023
2 parents 201dc39 + 6701f38 commit f866919
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
14 changes: 9 additions & 5 deletions src/sdk/v4/_fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export const fetcher = async <T extends object>(
return camelizeKeys(json) as T;
}

if (typeof fetch === 'undefined') {
throw new Error('fetch is not defined');
if (typeof globalThis.fetch !== 'function') {
throw new Error(
'Looks like there is no global fetch function. Take a look at https://quranjs.com/techniques#custom-fetcher for more info.'
);
}

// if there is no fetchFn, we use the global fetch
const res = await fetch(makeUrl(url, params));
const res = await globalThis.fetch(makeUrl(url, params));

if (!res.ok || res.status >= 400) {
throw new Error(`${res.status} ${res.statusText}`);
Expand All @@ -57,12 +59,14 @@ export const mergeApiOptions = (
options: MergeApiOptionsObject = {},
defaultOptions: Record<string, unknown> = {}
) => {
const clonedOptions = { ...options };

// we can set it to undefined because `makeUrl` will filter it out
if (options.fetchFn) options.fetchFn = undefined;
if (clonedOptions.fetchFn) clonedOptions.fetchFn = undefined;

const final: Record<string, unknown> = {
...defaultOptions,
...options,
...clonedOptions,
};

if (final.fields) {
Expand Down
27 changes: 27 additions & 0 deletions test/fetchFn.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { describe, expect, it } from 'vitest';
import fetch from 'cross-fetch';
import { quran } from '../src';

describe('Custom fetcher', () => {
it('should fail with no fetch', () => {
// @ts-expect-error - we are testing this
globalThis.fetch = undefined;

expect(() => quran.v4.chapters.findAll()).rejects.toThrowError(
/global fetch/
);
});

it('should not fail if you pass a fetchFn', () => {
// @ts-expect-error - we are testing this
globalThis.fetch = undefined;

expect(
quran.v4.chapters.findById('1', {
fetchFn: (url) => {
return fetch(url).then((res) => res.json());
},
})
).resolves.not.toThrow();
});
});
6 changes: 4 additions & 2 deletions test/setup.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import fetch from 'cross-fetch';
import { beforeAll, afterEach, afterAll } from 'vitest';
import { beforeAll, afterEach, afterAll, beforeEach } from 'vitest';
import { server } from '../mocks/server';

// Establish API mocking before all tests.
beforeAll(async () => {
server.listen();
});

beforeEach(async () => {
// we do this instead of passing a fetchFn every time
global.fetch = fetch;
globalThis.fetch = fetch;
});

// Reset any request handlers that we may add during the tests,
Expand Down

1 comment on commit f866919

@vercel
Copy link

@vercel vercel bot commented on f866919 Apr 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

api – ./docs

quranjs.vercel.app
quranjs.com
www.quranjs.com
api-quranjs.vercel.app
api-git-master-quranjs.vercel.app

Please sign in to comment.