Skip to content

Deprecate SubscriptionArgs and broaden ExecutionArgs #3295

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 7 commits into from
Oct 10, 2021

Conversation

yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Oct 7, 2021

Broaden ExecutionArgs to include all properties within SubscriptionArgs. Retain the SubscriptionArgs type for backwards compatibility, to be removed in the next major version.

This allows for better unification of subscription operations alongside queries and mutations as additional targets of execution.

Depends on #3294

@yaacovCR yaacovCR changed the title Broaden ExecutionArgs and deprecate SubscriptionArgs type Broaden ExecutionArgs type and deprecate SubscriptionArgs Oct 7, 2021
@yaacovCR yaacovCR force-pushed the broaden-execution-args branch 2 times, most recently from d5f133f to b92b2b4 Compare October 7, 2021 19:15
Copy link
Member

@IvanGoncharov IvanGoncharov left a comment

Choose a reason for hiding this comment

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

@yaacovCR Great PR 👍
Happy to merge it once you address all the points I left.
Please re-request review once you fix them.

@yaacovCR yaacovCR force-pushed the execute-root-fields branch from 04f6bd1 to 9d0a0a1 Compare October 9, 2021 17:28
@yaacovCR yaacovCR force-pushed the broaden-execution-args branch from b92b2b4 to 1b770ad Compare October 9, 2021 18:03
@yaacovCR yaacovCR requested a review from IvanGoncharov October 9, 2021 18:06
@yaacovCR yaacovCR changed the title Broaden ExecutionArgs type and deprecate SubscriptionArgs Deprecate SubscriptionArgs and broaden ExecutionArgs Oct 9, 2021
@yaacovCR yaacovCR force-pushed the execute-root-fields branch from 9d0a0a1 to ea6b79e Compare October 10, 2021 16:25
Broaden ExecutionArgs to include all properties within SubscriptionArgs. Retain the SubscriptionArgs type for backwards compatibility, to be removed in the next major version.

This allows for better unification of subscription operations alongside queries and mutations as additional targets of execution.
previously, this refactoring changed the `buildExecutionContext` method to store the default subscribeFieldResolver separately from the default fieldResolver within the ExecutionContext -- for clarity.

Unfortunately (and ironically!), we still used the fieldResolver property from the ExecutionContext instead of the new subscribeFieldResolver.
@yaacovCR yaacovCR force-pushed the broaden-execution-args branch from cfd03aa to 7ae9bdd Compare October 10, 2021 16:25
@IvanGoncharov IvanGoncharov added the PR: feature 🚀 requires increase of "minor" version number label Oct 10, 2021
@IvanGoncharov IvanGoncharov merged commit 05e2b21 into execute-root-fields Oct 10, 2021
@IvanGoncharov IvanGoncharov deleted the broaden-execution-args branch October 10, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature 🚀 requires increase of "minor" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants