Skip to content
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.

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

Since 2.6.0, FetchPolicy and react-apollo types that depend on it are broken #4847

Closed
adrienharnay opened this issue May 22, 2019 · 7 comments

Comments

@adrienharnay
Copy link

Intended outcome:
<Query fetchPolicy="cache-and-network" /> should be valid.

Actual outcome:

This change: cf069bc#diff-88b34d7add139a9c0e7d4c2b1944a8dc

broke all external types that depend on FetchPolicy, e.g.

export interface QueryOpts<TGraphQLVariables = OperationVariables> {
    ssr?: boolean;
    variables?: TGraphQLVariables;
    fetchPolicy?: FetchPolicy;
    ...

used by the Query class.

How to reproduce the issue:
Update apollo-client and write <Query fetchPolicy="cache-and-network" /> in a typescript project.

Versions

System:
    OS: macOS 10.14.4
  Binaries:
    Node: 10.15.3 - /usr/local/bin/node
    Yarn: 1.16.0 - ~/Documents/GitHub/brigad/front-end/node_modules/.bin/yarn
    npm: 6.4.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 74.0.3729.157
    Firefox: 65.0
    Safari: 12.1
  npmPackages:
    apollo: 2.12.2 => 2.12.2 
@alamothe
Copy link

Same issue. Does this mean cache-and-network is broken in 2.6.0?

@adrienharnay
Copy link
Author

It does work, only the types are mistaken 🙂

@benjamn benjamn self-assigned this May 22, 2019
@benjamn benjamn added this to the Release 2.6.0 milestone May 22, 2019
benjamn added a commit that referenced this issue May 22, 2019
Because they happen at build time, type errors are in a fundamentally
different category than runtime errors. We reserve the right to adjust our
types from time to time, and we rely on TypeScript to communicate those
changes to developers.

Type errors may require some action (hence this note in CHANGELOG.md) but
they communicate worthwhile information and are always worth fixing.

If you're not ready to deal with type-level changes, please use a
package-lock.json or yarn.lock file to pin the exact versions of your
dependencies.

Should help with #4847, along with the recent release of [email protected],
which includes this change to use WatchQueryFetchPolicy:
apollographql/react-apollo@63c2185
@benjamn
Copy link
Member

benjamn commented May 22, 2019

Thanks for reporting this issue, as we certainly did not intend this change to be so surprising. That said, this type change is meaningful, and adjusting your code to accommodate the new types (if necessary) is a good idea.

I've just released [email protected] which correctly uses WatchQueryFetchPolicy, and added a note to CHANGELOG.md about this change.

Please feel free to reopen this issue (or open a new issue) if the problem persists after updating to react-apollo@latest.

@benjamn
Copy link
Member

benjamn commented May 22, 2019

Here's the note from CHANGELOG.md:

  • The FetchPolicy type has been split into two types, so that passing cache-and-network to ApolloClient#query is now forbidden at the type level, whereas previously it was forbidden by a runtime invariant assertion:
    export type FetchPolicy =
      | 'cache-first'
      | 'network-only'
      | 'cache-only'
      | 'no-cache'
      | 'standby';
    
    export type WatchQueryFetchPolicy =
      | FetchPolicy
      | 'cache-and-network';
    The exception thrown if you ignore the type error has also been improved to explain the motivation behind this restriction.

    Issue #3130 (comment) and commit cf069bc7

@adrienharnay
Copy link
Author

Thanks that is perfect! Just wanted to make sure no other third parties I'm not aware of (react-apollo or others) made use of this as well 🙂

Have a great day!

@benjamn benjamn removed the 🏓 awaiting-contributor-response requires input from a contributor label May 22, 2019
@alamothe
Copy link

Should we then file an issue against react-apollo?

The issue is that Query component does not allow to pass cache-and-network anymore. Basically, the change in type here broke react-apollo (it needs to switch to WatchQueryFetchPolicy at appropriate places).

@adrienharnay
Copy link
Author

@alamothe a new version of react-apollo has been released, you should be able to update and the problem will be gone

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants