fix(graphql): prevent value fields from shadowing response input - #299
Open
amnn wants to merge 1 commit into
Open
fix(graphql): prevent value fields from shadowing response input#299amnn wants to merge 1 commit into
amnn wants to merge 1 commit into
Conversation
The `Response` derive generated one local binding per response field.
When a field named `value` came before another field, its binding
shadowed the `value` parameter containing the original JSON response, so
later fields tried to traverse the extracted field instead of the
response root, which often led to compilation (type) errors.
Before:
```rust
pub fn from_value(value: serde_json::Value) -> Result<Self, String> {
let value = {
let current = &value;
// extract value
};
let digest = {
let current = &value;
// incorrectly extracts from the value field
};
Ok(Self { value, digest })
}
```
After:
```rust
pub fn from_value(value: serde_json::Value) -> Result<Self, String> {
Ok(Self {
value: {
let current = &value;
// extract value
},
digest: {
let current = &value;
// extracts from the original response
},
})
}
```
This change initializes fields directly in the struct literal so field
names do not introduce local bindings.
## Test Plan
Adds a regression test with a `value` field followed by another
extraction.
```
$ cargo nextest run -p sui-graphql-macros --test extraction_test \
-- test_field_named_value_does_not_shadow_response_value
```
tpham-mysten
approved these changes
Aug 14, 2026
tpham-mysten
left a comment
Contributor
There was a problem hiding this comment.
Great catch! Thanks @amnn!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
Responsederive generated one local binding per response field. When a field namedvaluecame before another field, its binding shadowed thevalueparameter containing the original JSON response, so later fields tried to traverse the extracted field instead of the response root, which often led to compilation (type) errors.Before:
After:
This change initializes fields directly in the struct literal so field names do not introduce local bindings.
Test Plan
Adds a regression test with a
valuefield followed by another extraction.