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

release(Apollo): Update to Apollo v3 and GraphQL v15 #413

Merged
merged 1 commit into from
May 12, 2021

Conversation

nilgradisnik
Copy link
Contributor

@nilgradisnik nilgradisnik commented May 10, 2021

to: @williaster @alecklandgraf

Description

Update Apollo client to v3 and GraphQL v15

Motivation and Context

Apollo client v3 has been out for quite some time and we want to make sure Lunar is up to date. This release contains quite a few breaking changes so this PR is following the migration guide consolidating packages to @apollo/client and taking care of breaking changes.

This PR also updates GraphQL to v15 making sure we're up to date.

Also migrated to use lodash/fp.

Testing

Updated unit tests. Tested in Storybook.

Screenshots

N/A

Checklist

  • My code follows the style guide of this project.
  • I have updated or added documentation accordingly.
  • I have read the CONTRIBUTING document.

@@ -36,7 +33,7 @@ export default function addToList<Result, Vars = {}>(
}
}

set(nextResult, listPath, [...list, data]);
const nextResult = set(listPath, [...list, data], { ...queryResult });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

get and set function from lodash/fp have different order of arguments

@airbnb-bot
Copy link
Collaborator

Size Changes

Package Diff ESM Prev ESM CJS Prev CJS
apollo +1.1% 10.69 KB 10.58 KB 13.42 KB 13.82 KB

Compared to master. File sizes are unminified and ungzipped.

View raw build stats

Previous (master)

{
  "apollo": {
    "esm": 10832,
    "lib": 14147
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 584567,
    "lib": 733164
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}

Current

{
  "apollo": {
    "esm": 10946,
    "lib": 13746
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 584567,
    "lib": 733164
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}


return (cache: DataProxy) => {
return (cache: ApolloCache<Result>) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ApolloCache class implements DataProxy interface and we need it to evict cache

cache.evict({
id: cache.identify(resultRoot),
broadcast: false,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As stated here apollographql/apollo-client#6451 (comment) we need to evict cache when removing items from a list

nextResult,
const rootItem = listPath.split('.')[0];
// @ts-ignore
const resultRoot = queryResult[rootItem] as StoreObject;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if there's a better way to identify nested nodes but to evict cache we need at least the top level node ID for the cache.identify call below.

import prepareQuery from '../utils/prepareQuery';
import getQueryName from '../utils/getQueryName';

export default function addToList<Result, Vars = {}>(
docOrQuery: DocumentNode | DataProxy.Query<Vars>,
docOrQuery: DocumentNode | DataProxy.Query<Vars, Result>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DataProxy.Query requires both variables and result types now

}
}."
`;
exports[`addToList() errors if query does not exist in cache 1`] = `"\\"otherQuery\\" list \\"something.things\\" is not an array."`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cache.readQuery does not throw an error if there's no results anymore, it returns null instead. That's why the other error is thrown here.

@williaster
Copy link
Contributor

Ummm, we should no longer have commits in this repo as it's deprecated and has been merged into pineapple. @alecklandgraf do you disagree?

@williaster
Copy link
Contributor

If you still need to make changes to particular packages, we should maybe move those packages to be internal?

@nilgradisnik nilgradisnik marked this pull request as ready for review May 12, 2021 17:37
@nilgradisnik nilgradisnik requested a review from williaster as a code owner May 12, 2021 17:37
@nilgradisnik nilgradisnik merged commit ea0d95c into master May 12, 2021
@nilgradisnik nilgradisnik deleted the nil--apollo-update branch May 12, 2021 18:28
@alecklandgraf
Copy link
Contributor

@williaster I think this patch migrates the Apollo client to the version in pineapple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants