Skip to content

Proposal to solve dependency in GQL schema between QuoteGQL and WishlistGQL #413

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

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

prabhuram93
Copy link

Problem

Based on the uid changes, new/existing mutations in the future will be leveraging the EnteredOptionInput type.

input EnteredOptionInput {
    uid: ID!
    value: String!
}

Based on this architecture AddProductsToCart the type EnteredOptionInput is inferred to be in QuoteGQL module.

The same type is being used in WishlistGQL Wishlist for its mutations. This creates a dependency between WishlistGQL and QuoteGQL.

Solution

There a couple of ways to fix this.

Solution 1. Move the EnteredOptionInput to the GraphQL framework module, which will avoid cross dependencies across modules except for the framework module. (Added in the document)

Solution 2. Rename field names specific to the modules, possibly WishlistEnteredOptionInput and CartItemEnteredOptionInput and have the type on respective modules. This introduces a very minimum level of duplication but the provides the flexibility to update input types in the future. Flexibility plays a key role here, since modifying input types impacts the mutations in which the those types are being used in.

Solution 3. Create a new module to move this type to. This might be an overkill for just a type.

Requested Reviewers

@paliarush @akaplya @DrewML

@paliarush paliarush merged commit cb49b5d into magento:master Aug 4, 2020
prabhuram93 pushed a commit to rogyar/magento2 that referenced this pull request Aug 4, 2020
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.

2 participants