Skip to content

Pass fetchOptions to fetch call to match HttpLink behavior #302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Pass fetchOptions to fetch call to match HttpLink behavior #302

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10582aa
Pass fetchOptions to fetch call to match HttpLink behavior
Nov 5, 2022
30c93a9
Ignore fetchOptions body with warning and add AbortController tests
Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
315 changes: 309 additions & 6 deletions src/__tests__/restLink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2048,6 +2048,47 @@ describe('Query options', () => {
expect(credentials).toBe('my-credentials');
});

it('prioritizes fetchOptions credentials over context and setup credentials', async () => {
expect.assertions(2);

const credentialsMiddleware = new ApolloLink((operation, forward) => {
operation.setContext({
credentials: 'wrong-credentials-context',
});
return forward(operation).map(result => {
const { credentials } = operation.getContext();
expect(credentials).toBeDefined();
return result;
});
});

const link = ApolloLink.from([
credentialsMiddleware,
new RestLink({
uri: '/api',
credentials: 'wrong-credentials-setup' as RequestCredentials,
}),
]);

const post = { id: '1', title: 'Love apollo' };
fetchMock.get('/api/post/1', post);

await toPromise<Result>(
execute(link, {
operationName: 'post',
query: sampleQuery,
context: {
fetchOptions: {
credentials: "my-credentials",
}
}
}),
);

const credentials = fetchMock.lastCall()[1].credentials;
expect(credentials).toBe('my-credentials');
});

it('sets the fetch responses on context.restResponses', async () => {
expect.assertions(5);

Expand Down Expand Up @@ -2134,7 +2175,6 @@ describe('Query options', () => {
expect.objectContaining({ method: 'GET' }),
);
});

it('works without specifying a request method', async () => {
expect.assertions(1);

Expand Down Expand Up @@ -2165,6 +2205,41 @@ describe('Query options', () => {
expect.objectContaining({ method: 'GET' }),
);
});
it('prioritizes fetchOptions method over directive method', async () => {
expect.assertions(1);

const link = new RestLink({ uri: '/api' });

const post = { id: '1', title: 'Love apollo' };
fetchMock.post('/api/post/1', post);

const postTitleQuery = gql`
query postTitle {
post(input: { data: true }, id: "1") @rest(type: "Post", path: "/post/:id", method: "DELETE") {
id
title
}
}
`;

await toPromise<Result>(
execute(link, {
operationName: 'postTitle',
query: postTitleQuery,
variables: { id: '1' },
context: {
fetchOptions: {
method: "POST"
}
}
}),
);

const requestCall = fetchMock.calls('/api/post/1')[0];
expect(requestCall[1]).toEqual(
expect.objectContaining({ method: 'POST' }),
);
});
});

describe('headers', () => {
Expand Down Expand Up @@ -2282,6 +2357,47 @@ describe('Query options', () => {
}),
);
});
it('adds headers to the request from the fetchOptions', async () => {
const link = new RestLink({
uri: '/api',
});

const post = { id: '1', title: 'Love apollo' };
fetchMock.get('/api/post/1', post);

const postTitleQuery = gql`
query postTitle {
post(id: "1") @rest(type: "Post", path: "/post/:id") {
id
title
}
}
`;

await toPromise<Result>(
execute(link, {
operationName: 'postTitle',
query: postTitleQuery,
variables: { id: '1' },
context: {
fetchOptions: {
headers: {
authorization: "1234",
}
}
}
}),
);

const requestCall = fetchMock.calls('/api/post/1')[0];
expect({ headers: flattenHeaders(requestCall[1]) }).toEqual(
expect.objectContaining({
headers: expect.objectContaining({
authorization: '1234',
}),
}),
);
});
it('prioritizes context headers over setup headers', async () => {
expect.assertions(2);

Expand Down Expand Up @@ -2337,6 +2453,79 @@ describe('Query options', () => {
'setup: setup, in-context duplicate setup',
]);
});
it('prioritizes fetchOptions headers over context and setup headers', async () => {
expect.assertions(2);

const headersMiddleware = new ApolloLink((operation, forward) => {
operation.setContext({
headers: {
authorization: '1234',
// won't be overridden, will be duplicated because of headersToOverride
fetchOptionsContext: 'in-context duplicate fetchOptionsContext',
setup: 'in-context duplicate setup',
context: 'context',
},
headersToOverride: ['authorization'],
});
return forward(operation).map(result => {
const { headers } = operation.getContext();
expect(headers).toBeDefined();
return result;
});
});
const link = ApolloLink.from([
headersMiddleware,
new RestLink({
uri: '/api',
headers: {
authorization: 'no user',
fetchOptionsSetup: 'in-setup duplicate fetchOptionsSetup',
setup: 'setup'
},
}),
]);

const post = { id: '1', title: 'Love apollo' };
fetchMock.get('/api/post/1', post);

const postTitleQuery = gql`
query postTitle {
post(id: "1") @rest(type: "Post", path: "/post/:id") {
id
title
}
}
`;

await toPromise<Result>(
execute(link, {
operationName: 'postTitle',
query: postTitleQuery,
variables: { id: '1' },
context: {
fetchOptions: {
headers: {
authorization: 'fetch user',
fetchOptionsContext: 'fetchOptionsContext',
fetchOptionsSetup: 'fetchOptionsSetup',
fetchOptions: 'fetchOptions',
}
}
}
}),
);

const requestCall = fetchMock.calls('/api/post/1')[0];
expect(orderDupPreservingFlattenedHeaders(requestCall[1])).toEqual([
'accept: application/json',
'authorization: fetch user',
'context: context',
'fetchoptions: fetchOptions',
'fetchoptionscontext: in-context duplicate fetchOptionsContext, fetchOptionsContext',
'fetchoptionssetup: in-setup duplicate fetchOptionsSetup, fetchOptionsSetup',
'setup: setup, in-context duplicate setup',
]);
});
it('respects context-provided header-merge policy', async () => {
expect.assertions(2);

Expand Down Expand Up @@ -2389,6 +2578,14 @@ describe('Query options', () => {
operationName: 'postTitle',
query: postTitleQuery,
variables: { id: '1' },
context: {
fetchOptions: {
headers: {
authorization: "fetchOptions",
fetchOptions: "fetchOptions",
}
}
}
}),
);

Expand All @@ -2399,6 +2596,7 @@ describe('Query options', () => {
authorization: 'initial setup',
setup: 'setup',
context: 'context',
fetchoptions: "fetchOptions",
}),
}),
);
Expand Down Expand Up @@ -2497,6 +2695,111 @@ describe('Query options', () => {
]);
});
});
describe('body', () => {
it('prioritizes fetchOptions body over directive body', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does fetchOptions body come from? What is it for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a lot of interplaying features, and I'm not super comfortable with blindly accepting a body via a context parameter without understanding the spec.

The abortcontroller is interesting and makes sense to me, but I don't think the body should be passed through?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think the body option is necessarily the most useful thing, but I included it for completeness. I was copying the behavior in the HTTP Link, which simply passes all fetch options to the fetch function. I think it makes sense to include this option so no one tries to use it and gets confused by it not working (which is the state I was in when I tried to use an abort controller). It should not break any existing behavior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The problem is that we can't guarantee other Links aren't accidentally injecting a body, and if other links do inject a body, we can't parse the results or otherwise handle it.

I get why AbortController makes sense, but if we use the body, we're actually building apollo-link-rest into a really inefficient clone of HTTPLink that doesn't do the right thing?

Maybe I'm confused as to how this body is used normally and when it would be expected to be passed to this link.

const link = new RestLink({
uri: '/api',
});

const post = { id: '1', title: 'Love apollo' };
fetchMock.post('/api/posts/new', post);

const createPostMutation = gql`
fragment Item on any {
name: String
}

fragment PublishablePostInput on REST {
id: String
title: String
items {
...Item
}
}

mutation publishPost($input: PublishablePostInput!) {
publishedPost(input: $input)
@rest(type: "Post", path: "/posts/new", method: "POST") {
id
title
items
}
}
`;

await toPromise<Result>(
execute(link, {
operationName: 'postTitle',
query: createPostMutation,
variables: { input: { id: '1', title: 'title', items: [{ name: 'name' }] } },
context: {
fetchOptions: {
body: 'my-body'
}
}
}),
);

const body = fetchMock.lastCall()[1].body;
expect(body).toBe('"my-body"');
});
})
describe('fetchOptions', () => {
it('passes all fetchOptions to the fetch call', async () => {
const link = new RestLink({
uri: '/api',
});

const post = { id: '1', title: 'Love apollo' };
fetchMock.get('/api/post/1', post);

const postTitleQuery = gql`
query postTitle {
post(id: "1") @rest(type: "Post", path: "/post/:id") {
id
title
}
}
`;

const abortController = new AbortController()
await toPromise<Result>(
execute(link, {
operationName: 'postTitle',
query: postTitleQuery,
variables: { id: '1' },
context: {
fetchOptions: {
cache: "no-cache",
integrity: "integrity-hash",
keepalive: true,
mode: "same-origin",
redirect: "follow",
referrer: "referrer",
referrerPolicy: "origin",
signal: abortController.signal,
window: null,
}
}
}),
);

const requestCall = fetchMock.calls('/api/post/1')[0];
expect(requestCall[1]).toEqual(
expect.objectContaining({
cache: "no-cache",
integrity: "integrity-hash",
keepalive: true,
mode: "same-origin",
redirect: "follow",
referrer: "referrer",
referrerPolicy: "origin",
signal: abortController.signal,
window: null,
}),
);
});
})
});

describe('Mutation', () => {
Expand Down Expand Up @@ -3454,7 +3757,7 @@ describe('Mutation', () => {
//mocking FileList object
const mockFileList = Object.create(FileList.prototype);
Object.defineProperty(mockFileList, 'item', {
value: function(number: number) {
value: function (number: number) {
return mockFileList[number];
},
writable: false,
Expand Down Expand Up @@ -3577,7 +3880,7 @@ describe('validateRequestMethodForOperationType', () => {
expect.assertions(1);
expect(() =>
validateRequestMethodForOperationType('GIBBERISH', 'mutation'),
).toThrowError('"mutation" operations do not support that HTTP-verb');
).toThrowError('"mutation" operations do not support the GIBBERISH method; please use one of GET, POST, PUT, PATCH, DELETE');
});
});
describe('for operation type "subscription"', () => {
Expand Down Expand Up @@ -3625,9 +3928,9 @@ describe('export directive', () => {
} catch (e) {
expect(e.message).toBe(
'Missing parameters to run query, specify it in the query params or use ' +
'an export directive. (If you need to use ":" inside a variable string' +
' make sure to encode the variables properly using `encodeURIComponent' +
'`. Alternatively see documentation about using pathBuilder.)',
'an export directive. (If you need to use ":" inside a variable string' +
' make sure to encode the variables properly using `encodeURIComponent' +
'`. Alternatively see documentation about using pathBuilder.)',
);
}
});
Expand Down
Loading