Conversation
…lver # Conflicts: # docs/master/digging-deeper/extending-lighthouse.md # tests/Integration/Models/PropertyAccessTest.php
spawnia
added
enhancement
spawnia
marked this pull request as draft
June 12, 2025 20:37
…field-resolver # Conflicts: # CHANGELOG.md
…field-resolver # Conflicts: # CHANGELOG.md # docs/master/digging-deeper/extending-lighthouse.md
🤖 Generated with Claude Code
🤖 Generated with Claude Code
spawnia
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Uninitialized public typed model properties can currently cause GraphQL execution errors.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Introduces a Lighthouse default field resolver that supports public Eloquent model properties while avoiding duplicate accessor reads.
Changes:
- Adds model-aware attribute/property resolution.
- Adds regression coverage for models and collections.
- Documents the resolver’s behavioral changes.
| File | Description |
|---|---|
src/LighthouseServiceProvider.php |
Registers and implements the resolver. |
tests/Integration/DefaultFieldResolverTest.php |
Tests collection key resolution. |
tests/Integration/Models/PropertyAccessTest.php |
Tests model properties, accessors, and framework-property hiding. |
tests/Utils/Models/User.php |
Extends the model fixture for resolver tests. |
docs/master/digging-deeper/extending-lighthouse.md |
Documents resolver customization. |
UPGRADE.md |
Describes upgrade behavior changes. |
CHANGELOG.md |
Records the resolver override. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ($root instanceof Model) { | ||
| $property = $root->getAttribute($fieldName); | ||
| if ($property === null && static::isApplicationProperty($root, $fieldName)) { | ||
| $property = $root->{$fieldName}; |
| ### Default field resolver changed | ||
|
|
||
| The default field resolver was changed from `GraphQL\Executor\Executor::defaultFieldResolver()` to `Nuwave\Lighthouse\LighthouseServiceProvider::defaultFieldResolver()`. | ||
| The new default fields resolver is expected to be mostly compatible with the previous one, but introduces some behavior changes when resolving models: |
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.
Resolves #2191
Resolves #1671
Resolves #2687
Changes
Fields on Eloquent models ignore public PHP properties and call accessors twice.
Lighthouse now installs its own default field resolver that reads each attribute once and falls back to public properties of the model.
This is the Lighthouse-side alternative to webonyx/graphql-php#1960, which adds an opt-in marker interface to graphql-php instead.
Eloquent's own properties stay hidden, avoiding the leaks found in the graphql-php survey
The survey in webonyx/graphql-php#1960 (comment) lists what an unconditional property fallback breaks.
The resolver only reads a property when it is public, not static, and declared outside
Illuminate\.Names that exist on
Eloquent\Model, such asexists,incrementing,timestamps,wasRecentlyCreatedandpreventsLazyLoading, resolve tonull, also when a model redeclares them.Values other than models, such as Laravel collections, still go through
Utils::extractKey(), soCollection::__getis never called.A property fills a null attribute, a non-null attribute wins
When a model has an attribute and a public property of the same name, the resolver returns the attribute unless it is
null.This matches the fallback order of webonyx/graphql-php#1960.
Breaking changes
Fields that resolved to
nullbecause they matched a public model property now return the property value.Null accessors run once instead of twice.
UPGRADE.mdlists both.🤖 Generated with Claude Code