Skip to content

Fix @with lazy loading for duplicate models - #2803

Closed
jaulz wants to merge 4 commits into
nuwave:masterfrom
jaulz:fix-with-duplicate-models
Closed

jaulz wants to merge 4 commits into
nuwave:masterfrom
jaulz:fix-with-duplicate-models

Conversation

@jaulz

@jaulz jaulz commented Oct 16, 2026

Copy link
Copy Markdown
Contributor
  • Added or updated tests
  • Documented user facing changes

Changes

RelationBatchLoader keeps one parent per model key: when the same model is reached more than once under a field (e.g. the same Post under two Tags), each later instance overwrites the previous one in $parents, so only the last instance receives the eager loaded relation.

@with and @withCount discard the batch loader's result and run the resolver on their own parent, assuming the relation (or count attribute) is now present on it. That only holds for the last instance; every other instance of the same model lazy loads the relation in the resolver, one query per duplicate. With Model::preventLazyLoading() enabled, it throws instead.

This adds a hydrate() hook to WithRelationDirective that receives the batch loader's result before the resolver runs:

  • @with sets the relation on the parent (the first level of a dotted relation, which holds the nested ones)
  • @withCount sets the {relation}_count attribute on the parent

Breaking changes

None. Only subclasses of WithRelationDirective that already define a hydrate() method with a different signature would conflict.

Changelog

Fixed

  • Load @with and @withCount results onto every instance of a model reached multiple times in one query

@spawnia
spawnia requested a balanced review from Copilot October 25, 2026 11:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@spawnia

spawnia commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

🤖 @jaulz #2807 fixes the same bug one level lower: RelationBatchLoader keys parents by object ID, so every instance of a duplicated row gets the relation.
Would that solve your problem too?

It also covers paginated @hasMany on duplicates, which this PR leaves out.
If #2807 works for you, I would merge that one and close this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Copied count attributes remain dirty on duplicate parents, causing subsequent saves to attempt updating nonexistent database columns.

1 open issue

Review effort: Balanced

protected function storeLoadedResult(Model $parent, mixed $result): void
{
// Same attribute name as CountModelsLoader::extractCount() reads
$parent->setAttribute(Str::snake("{$this->relation()}_count"), $result);
@jaulz

jaulz commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@spawnia that would help as well. Thanks for taking care of it!

@jaulz jaulz closed this Oct 26, 2026
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.

3 participants