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

Deprecate SubscriptionArgs and broaden ExecutionArgs #3295

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 changed the title Broaden ExecutionArgs type and deprecate SubscriptionArgs Deprecate SubscriptionArgs and broaden ExecutionArgs Oct 9, 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.
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.
@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.

None yet

3 participants