@orta => Remove all legacy loader, remove couple rewire uses
#954
Conversation
| it("fans out all the requests", () => { | ||
| const loader = jest | ||
| .fn() | ||
| .mockReturnValueOnce( |
There was a problem hiding this comment.
The first call is to retrieve the count, and then we fan out all required pages (for simplicity just resolving with an empty body as this helper doesn't care what is resolved).
| @@ -110,8 +110,17 @@ const PartnerShowType = new GraphQLObjectType({ | |||
| description: "The slug or ID of an artist in the show.", | |||
| }, | |||
| }, | |||
| resolve: ({ id, partner }, options) => { | |||
There was a problem hiding this comment.
I missed these 2 when I swept thru this file the other day.
| total.onCall(0).returns(Promise.resolve(2)) | ||
| rootValue.partnerShowArtworksLoader = sinon | ||
| .stub() | ||
| .returns(Promise.resolve({ headers: { "x-total-count": 2 } })) |
There was a problem hiding this comment.
I’d also suggest getting rid of sinon over builtin jest stubs.
There was a problem hiding this comment.
👍 can I tackle that in a separate issue/cleanup? There are...a lot of those :)
There was a problem hiding this comment.
I would recommend it, as a separate issue,there's a tool (jest-codemods) that might help here
There was a problem hiding this comment.
It's a WIP, skovhus/jest-codemods#68 (comment)
There was a problem hiding this comment.
Oh absolutely, it was only an observation.
| @@ -25,6 +24,7 @@ import { | |||
| GraphQLBoolean, | |||
| } from "graphql" | |||
| import { allViaLoader } from "../lib/all" | |||
| import { totalViaLoader } from "../lib/loaders/legacy/total" | |||
There was a problem hiding this comment.
Oh, yea I will eventually clean up the legacy total, and move the helper out of legacy/ directory (which won't exist anymore).
|
Yep, both babel-rewire and jest modules want to have control over the module system, and that introduces conflicts, so until babel-rewire is completely removed we can't use jest mocks |
|
Yep, looks 👍 to me |
As I'm working thru remaining legacy data loaders (home page ones coming next!), I figured it's a good idea to PR early and often. Much like voting.
So this removes a legacy
allhelper, that was being used like:gravity.all("artworks"), which would continue paginating the API until exhausted. We have aallViaLoaderhelper which is used in a couple places instead (and an open issue to remove the client code in Force which requests that since it's a horrible pattern).Also removes some rewire/stubbing via explicit DI of the loaders. Definitely doing a lot of this in these PRs, @alloy mentions we can't easily have both Rewire and Jest stubs in the same project, and so it'd be preferable to remove rewire entirely?