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

Default plan resolver should use .get(...) if possible #2306

Open
eugeneduda opened this issue Jan 9, 2025 · 5 comments
Open

Default plan resolver should use .get(...) if possible #2306

eugeneduda opened this issue Jan 9, 2025 · 5 comments
Labels

Comments

@eugeneduda
Copy link

Summary

When I try to wrap a step result in object I get an error:

Error: The step returned by 'createAsset.asset' is not compatible with the GraphQL object type 'Asset': Expected a PgSelectSingleStep, PgInsertSingleStep, PgUpdateSingleStep or PgDeleteSingleStep, however we received 'Access{2}➊<35.asset>[42]'.

Steps to reproduce

A code example:

makeExtendSchemaPlugin((build) => {
   const { assets } = build.input.pgRegistry.pgResources;
   if (!assets) throw new Error("...");

   return {
     typeDefs: gql`
       type CreateAssetPayload {
         asset: Asset!
       }

       CreateAssetInput {
        ...
       }
  
       extend type Mutation {
         createAsset(input: CreateAssetInput!): CreateAssetPayload
       }
     `,
     plans: {
       Mutation: {
         createAsset() {
           const $id = withPgClient(...);

           return object({ asset: assets.get({ id: $id }) });
         }
       }
     }
   }
});

There is no problem when it returns assets.get({ id: $id })

The issues will be fixed if you add the following plan:

CreateAssetPayload: { 
    asset($payload) { 
        return $payload.get(‘asset’);
    } 
}

Expected results

You don't have to add the CreateAssetPayload plan resolver

Additional context

My PostGraphile version is 5.0.0-beta.35

@github-project-automation github-project-automation bot moved this to 🌳 Triage in V5.0.0 Jan 9, 2025
@benjie benjie changed the title Wrap response in object Default plan resolver should use .get(...) if possible Jan 9, 2025
@benjie
Copy link
Member

benjie commented Jan 9, 2025

Hmmm... This is weird because it should work:

function makeDefaultPlan(fieldName: string) {
return (
$step: ExecutableStep & { get?: (field: string) => ExecutableStep },
) =>
typeof $step.get === "function"
? $step.get(fieldName)
: access($step, [fieldName]);
}

const planResolver =
rawPlanResolver ??
(resolver ? undefined : makeDefaultPlan(fieldName));

@benjie
Copy link
Member

benjie commented Jan 9, 2025

Are you using makeWrapPlansPlugin perchance? I don't see it in your "reproduction", so I assume not? I've not tried to reproduce it yet, because your reproduction is just an excerpt rather than something runnable.

@benjie
Copy link
Member

benjie commented Jan 9, 2025

I think you must be using makeWrapPlansPlugin and you're wrapping this resolver on the mutation payload for some reason; I can see that the fallback in makeWrapPlansPlugin is the old-style default plan which doesn't use get:

plan: oldPlan = EXPORTABLE(
(access, fieldName) =>
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
($obj: import("grafast").ExecutableStep) =>
access($obj, fieldName),
[access, fieldName],
),

Please confirm whether or not this is the case.

Either way, please actually create and share a minimal reproduction in future and test that it reproduces the issue rather than just sharing a small excerpt of what you think is causing it; it will likely save me significant time. We're lucky I could intuit it this time around... assuming I'm correct.

@eugeneduda
Copy link
Author

eugeneduda commented Jan 9, 2025

Yes. I think you are right. I will make an exact reproduction of the error and reopen the issue

@github-project-automation github-project-automation bot moved this from 🌳 Triage to ✅ Done in V5.0.0 Jan 9, 2025
@benjie benjie reopened this Jan 9, 2025
@github-project-automation github-project-automation bot moved this from ✅ Done to 🌱 In Progress in V5.0.0 Jan 9, 2025
@benjie
Copy link
Member

benjie commented Jan 9, 2025

Either way, the makeWrapPlansPlugin default resolver needs fixing so I'll leave it open.

@benjie benjie moved this from 🌱 In Progress to 🦟 Mayfly in V5.0.0 Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🦟 Mayfly
Development

No branches or pull requests

2 participants