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

Add GetInputExtension method #3615

Merged
merged 6 commits into from Apr 26, 2023
Merged

Add GetInputExtension method #3615

merged 6 commits into from Apr 26, 2023

Conversation

sungam3r
Copy link
Member

Why does IResolveFieldContext.InputExtensions has IReadOnlyDictionary<string, object?> type instead of Inputs? Because of this I have to cast extensions

 // actually context.InputExtensions is of type GraphQL.Inputs
                if (context.InputExtensions is not IDictionary<string, object?> values || values.Count == 0)
                    return null;

@github-actions github-actions bot added the test Pull request that adds new or changes existing tests label Apr 25, 2023
@Shane32 Shane32 linked an issue Apr 25, 2023 that may be closed by this pull request
@sungam3r
Copy link
Member Author

Change IReadOnlyDictionary<string, object?> to Inputs for v8?

@Shane32
Copy link
Member

Shane32 commented Apr 25, 2023

Change IReadOnlyDictionary<string, object?> to Inputs for v8?

No, why? Because you had a cast in there? Just change the code to match on IReadOnlyDictionary.

@sungam3r
Copy link
Member Author

sungam3r commented Apr 26, 2023

No, why? Because you had a cast in there?

Because IReadOnlyDictionary<T,U> does not inherit IDictionary<T,U> and vice versa.

Just change the code to match on IReadOnlyDictionary.

Does not compile. I rewrote it and added comment.

@sungam3r sungam3r self-assigned this Apr 26, 2023
@sungam3r sungam3r added the enhancement New feature or request label Apr 26, 2023
@sungam3r sungam3r requested a review from Shane32 April 26, 2023 17:03

lock (context.OutputExtensions)
private static object? GetByPath(IDictionary<string, object?>? dictionary, string path, bool useLock)
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest changing the signature here to IReadOnlyDictionary here and the is IDictionary line. It probably makes little difference, as the ImmutableDictionary class and even the brand-new FrozenDictionary class supports IDictionary as well as IReadOnlyDictionary.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@codecov-commenter
Copy link

Codecov Report

Merging #3615 (d2fd2b8) into master (1ae26bf) will increase coverage by 0.00%.
The diff coverage is 95.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #3615   +/-   ##
=======================================
  Coverage   83.91%   83.92%           
=======================================
  Files         381      381           
  Lines       16876    16888   +12     
  Branches     2715     2717    +2     
=======================================
+ Hits        14162    14173   +11     
- Misses       2068     2069    +1     
  Partials      646      646           
Impacted Files Coverage Δ
...solveFieldContext/ResolveFieldContextExtensions.cs 86.66% <94.44%> (+2.70%) ⬆️
src/GraphQL/Execution/DocumentExecuter.cs 93.54% <100.00%> (-0.07%) ⬇️

... and 1 file with indirect coverage changes

@sungam3r sungam3r merged commit f3fa7d5 into master Apr 26, 2023
8 checks passed
@sungam3r sungam3r deleted the ext branch April 26, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request test Pull request that adds new or changes existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide GetInputExtension method
3 participants