fix(types): use correct type for headers #9042
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When passing the headers property to `createHttpLink` one might think that it is ok to pass them as `Headers` interface, but that doesn't work because there is several places in the code that expect headers to be passed as an object.
|
@alias-mac: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
|
Sorry for the ping @benjamn, @brainkim, @jcreighton and @hwillson, but if I rebase this, can it be merged? Thanks! |
|
Any chance to get this reviewed and merged? Let me know what changes you don't agree with. Thanks! |
jpvajda
added
🏓 awaiting-contributor-response
jerelmiller
approved these changes
Nov 18, 2022
There was a problem hiding this comment.
This looks like a positive change to me and I see no reason not to include this. Thanks for your patience @alias-mac!
|
@alias-mac I just merged in |
* Changelog * Add only to http-link doc * Batch, httpLink constructor, and adv. networking
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…raphql#10144) keyArgs must be set to false to properly merge multiple requests when paging because "cursor" is passed in as a query variable (even though it isn't referenced in the merge function)
* updates roadmap nov 2022 * roadmap updates dec 2022 * added milestone * Update ROADMAP.md * Update ROADMAP.md Co-authored-by: Jeff Auriemma <bignimbus@users.noreply.github.com>
* Introduce changesets * Add check-prerelease workflow * Add repository check to prevent PR creation on forks
* Better handle cached data with deferred queries When using `useQuery` with deferred queries that already have cache data written, the initial chunk of data would overwrite anything in the cache, which meant cached data for deferred chunks would disappear. This is now better handled by merging existing cache data with the initial deferred chunk to ensure a complete result set is still returned. * Add changeset
* chore(deps): update cimg/node docker tag to v19 * chore: update snapshot test when run via node v19 * chore: move preinstall script steps to test:memory Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: alessia <alessia@apollographql.com>
…apollographql#10454) * Add `onError` and clarify `shouldResubscribe` * Remove extra commas Co-authored-by: Jeff Auriemma <bignimbus@users.noreply.github.com>
…raphql#10455) * Set Stale Action debug to false * Update operations-per-run from 100 to 500
* Allow declaration merging for DefaultContext * Use DefaultContext instead of any * Fix test * Add changeset Co-authored-by: Alessia Bellisario <alessia@apollographql.com> Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
…raphql#10321) * chore: adds failing test for issue 10317 * chore: reset data to undefined on refetch with errorPolicy: none * chore: use watchQuery.errorPolicy for default value * chore: clean up test * chore: adds changeset * fix: shouldNotify should be false if errorPolicy: none and missing errors * fix: do not return cache data if errorPolicy none on refetch and missing errors * chore: undo whitespace removal * chore: add comment and test
* chore: fix netlify ignore script * chore: update node version, add ignore script * fix: specify docs folder * fix: assume base of docs * test change detection * revert docs change
Addresses apollographql#10457 Passes `getServerSnapshot` to `useSyncExternalStore` so that it doesn't trigger a `Missing getServerSnapshot` error when using `useFragment_experimental` on the server. Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
When passing the headers property to `createHttpLink` one might think that it is ok to pass them as `Headers` interface, but that doesn't work because there is several places in the code that expect headers to be passed as an object.
🦋 Changeset detectedLatest commit: 1f70e59 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
3 tasks
|
Hi @alias-mac 👋 I went to add a changeset here so we can get this merged in and made a mess of the git history as you can see :) I'm going to close this one out and open a new PR that includes your commit, thanks! |
jerelmiller
pushed a commit
that referenced
this pull request
Jan 23, 2023
* fix(types): use correct type for headers When passing the headers property to `createHttpLink` one might think that it is ok to pass them as `Headers` interface, but that doesn't work because there is several places in the code that expect headers to be passed as an object. * chore: fix TS errors * chore: adds changeset Co-authored-by: Filipe Guerra <alias.mac@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When passing the headers property to
createHttpLinkone might think that it is ok to pass them asHeadersinterface, but that doesn't work because there is several places in the code that expect headers to be passed as an object.Basically the main problem is using code like:
Keep in mind that the code above is simplified and the idea is that headers passed in are of type
Headers.TypeScript won't complain because Apollo accepts
anyin this type, but that isn't quite true (based on the code).I believe that the type here should be stricter to what Apollo is actually expecting to be passed in, and perhaps revisit this later to support
Headersinterface.Checklist: