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

feat: update delivery function handler interface #327

Merged
merged 3 commits into from
Aug 28, 2023
Merged

Conversation

BasKiers
Copy link
Contributor

@BasKiers BasKiers commented Aug 24, 2023

In the PH on Tuesday we discussed the following changes

  • Revert the handler map to a handler function using type unions
  • Add a helper to make "type save" event handlers
  • Add a isIntrospectionQuery boolean to the graphql.query event.
  • Allow GraphQLQueryArgument to be a Record<string, string> to allow for multi-argument GraphQLQueryField calls. The record key would be the argument name, the key value a JSON Pointer (not set in stone, we might use a different string format) that points to the JSON property that needs to be extracted from the field value and provided as the argument value.

@BasKiers BasKiers requested a review from a team as a code owner August 24, 2023 09:56
@andipaetzold
Copy link
Contributor

Can you update the PR title? "PH discussion" is not suitable for a public npm package changelog ;)

Copy link
Contributor

@kdamball kdamball left a comment

Choose a reason for hiding this comment

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

Will also need a change the template code, right?

query: string
isIntrospectionQuery: boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be part of Field mapping too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I'm following. This boolean is used to indicate wether the query being sent for this event is an introspection query. An app can use this to easily overwrite where the schema is loaded from, for instance by loading it from the file system instead of passing the introspection query along to the 3rd party GraphQL API.

Copy link
Contributor

@anho anho left a comment

Choose a reason for hiding this comment

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

Can we adjust the graphQLQueryArgument of GraphQLFieldTypeMapping as well?

src/requests/typings/deliveryFunction.ts Outdated Show resolved Hide resolved
@BasKiers BasKiers changed the title feat: update types based on PH discussion feat: update delivery function handler type interface Aug 24, 2023
@BasKiers BasKiers changed the title feat: update delivery function handler type interface feat: update delivery function handler interface Aug 24, 2023
@BasKiers BasKiers requested review from kdamball and anho August 24, 2023 11:20
Copy link
Contributor

@anho anho left a comment

Choose a reason for hiding this comment

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

GraphQLFieldTypeMappingResponse should be the following:

export type GraphQLFieldTypeMappingResponse = {
  namespace: string
  fields: GraphQLFieldTypeMapping[]
}

@BasKiers BasKiers enabled auto-merge (squash) August 28, 2023 09:41
@BasKiers BasKiers merged commit c31c30b into master Aug 28, 2023
@BasKiers BasKiers deleted the monet-1334 branch August 28, 2023 09:41
contentful-automation bot added a commit that referenced this pull request Aug 28, 2023
# [2.4.0](v2.3.0...v2.4.0) (2023-08-28)

### Features

* update delivery function handler interface ([#327](#327)) ([c31c30b](c31c30b))
@contentful-automation
Copy link
Contributor

🎉 This PR is included in version 2.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants