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

types: add option for strict connection types #1504

Merged
merged 5 commits into from Jul 26, 2023
Merged

types: add option for strict connection types #1504

merged 5 commits into from Jul 26, 2023

Conversation

shrouxm
Copy link
Contributor

@shrouxm shrouxm commented May 4, 2023

This PR adds a strict_types meta option to the relay Connection class to simplify the use case identified in this issue: graphql-python/graphene-django#901

Default of False is the old behavior, and when set to True generated TConnection types will be typed as edges: [TEdge!]! instead of edges: [TEdge]! and TEdge will have node: T! instead of node: T.

This avoids duplicating logic and string descriptions for subclasses which just want to implement this common behavior. If this behavior is deemed desirable by default it would be trivial to flip the flag's default or remove it entirely. It only make the types stricter so it oughtn't break any existing code (though it will cause unexpected changes to user's schema's nonetheless, which is why I kept the default to False).

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (6b8cd2d) 96.01% compared to head (035f19c) 96.01%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1504   +/-   ##
=======================================
  Coverage   96.01%   96.01%           
=======================================
  Files          51       51           
  Lines        1755     1755           
=======================================
  Hits         1685     1685           
  Misses         70       70           
Impacted Files Coverage Δ
graphene/relay/connection.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Can you please add some test cases for this, testing the schema generation? 😊

@shrouxm
Copy link
Contributor Author

shrouxm commented Jun 8, 2023

@erikwrede does that look like what you meant?

@shrouxm
Copy link
Contributor Author

shrouxm commented Jul 25, 2023

@erikwrede it looks like the checks will never complete because the python 3.6 test workflow no longer exists after https://github.com/graphql-python/graphene/pull/1507/files but the GitHub repo settings still require that workflow to run

@erikwrede erikwrede merged commit ea7ccc3 into graphql-python:master Jul 26, 2023
8 checks passed
shrouxm added a commit to techmatters/terraso-backend that referenced this pull request Jul 26, 2023
shrouxm added a commit to techmatters/terraso-backend that referenced this pull request Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants