-
Notifications
You must be signed in to change notification settings - Fork 344
spec-compliant GET support #490
Changes from all commits
ad49e93
ddc0f14
ec5a059
95caecf
fffde25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Change log | ||
|
||
### v0.2.0 | ||
- rename serializeFetchBody to serializeFetchParameter and take a label argument |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,14 @@ export interface UriFunction { | |
(operation: Operation): string; | ||
} | ||
|
||
// The body of a GraphQL-over-HTTP-POST request. | ||
export interface Body { | ||
query?: string; | ||
operationName?: string; | ||
variables?: Record<string, any>; | ||
extensions?: Record<string, any>; | ||
} | ||
|
||
export interface HttpOptions { | ||
/** | ||
* The URI to use when fetching operations. | ||
|
@@ -220,7 +228,7 @@ export const selectHttpOptionsAndBody = ( | |
|
||
//The body depends on the http options | ||
const { operationName, extensions, variables, query } = operation; | ||
const body = { operationName, variables }; | ||
const body: Body = { operationName, variables }; | ||
|
||
if (http.includeExtensions) (body as any).extensions = extensions; | ||
|
||
|
@@ -233,18 +241,18 @@ export const selectHttpOptionsAndBody = ( | |
}; | ||
}; | ||
|
||
export const serializeFetchBody = body => { | ||
let serializedBody; | ||
export const serializeFetchParameter = (p, label) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jaydenseric This is going to be a breaking change for [email protected] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this OK? I thought this hadn't been released yet so it was an OK interface to break without documenting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's ok if the change is documented in a changelog entry, and a new version of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I didn't realize Evans had published a non alpha version of the package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Evans, should I just revert the name change or something, or bump to 0.2.0? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you keep the breaking changes, please bump to v1.0.0, breaking changes in semver minor releases are the devil. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @glasser We can bump to v1.0.0, since it will make Jayden's life easier. I'm not worried about having the version number bloat and it might force better practice. The plan for apollo-link-common-http was to be a semi private API(without documentation due to overhead) that developers of links can look at if they want to align with apollo-link-http and apollo-link-batch-http. In that goal I think attempting to follow SemVer would be make keeping up with the changes easier There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I am wrong ^. Let's do v0.2, since we are pre v1.0 SemVer says that a breaking change should be a minor version. Also it looks like we should be okay for Jayden's upload-client There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All 3 things should be assumed to be true for every npm package that can be publically installed. There are only upsides and no downsides to bumping the major version number if there is ever a breaking change. FB in particular is the absolute worst for this. |
||
let serialized; | ||
try { | ||
serializedBody = JSON.stringify(body); | ||
serialized = JSON.stringify(p); | ||
} catch (e) { | ||
const parseError = new Error( | ||
`Network request failed. Payload is not serializable: ${e.message}`, | ||
`Network request failed. ${label} is not serializable: ${e.message}`, | ||
) as ClientParseError; | ||
parseError.parseError = e; | ||
throw parseError; | ||
} | ||
return serializedBody; | ||
return serialized; | ||
}; | ||
|
||
//selects "/graphql" by default | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may be able to have Body be an interface that extends Partial, see this SO.
GraphQLRequest
is exported fromapollo-link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query
is a string rather than a DocumentNode (and there's nocontext
), so I don't think that'll work?In my experience over the entire open source and commercial Apollo codebase, guessing what type is going to be in a field or variable called
query
is the most challenging part :)