From 3e16091212780d2aafce23d1b6b397dfe13d6029 Mon Sep 17 00:00:00 2001 From: benkhalife Date: Tue, 30 Jun 2026 15:43:02 +0330 Subject: [PATCH 1/2] fix(eloquent): make paginate() honour with() eager loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EagerBuilder only declared get() and first() as terminal methods that trigger Model::eagerLoad(). paginate() fell through __call() straight to the underlying Query\Builder::paginate(), which runs its own get() internally and never touches eager loading — silently dropping any with() chained before it. Practical effect: Model::with('relation')->paginate(...) returned a page whose rows had the relation unloaded. Accessing it afterwards fell back to lazy loading per row, reproducing full N+1 (e.g. 12 queries for a 10-row page instead of 3) with no error to signal it. Adds EagerBuilder::paginate(), which runs the page query then calls Model::eagerLoad() on $page->data before returning — mirroring what get() and first() already do. --- src/Eloquent/EagerBuilder.php | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Eloquent/EagerBuilder.php b/src/Eloquent/EagerBuilder.php index 84f9943..711c3dd 100644 --- a/src/Eloquent/EagerBuilder.php +++ b/src/Eloquent/EagerBuilder.php @@ -71,6 +71,31 @@ public function first(): ?Model return $loaded->first(); } + /** + * Paginate the results and eager-load all specified relations on + * the current page. + * + * Without this override, paginate() would fall through __call() to + * Query\Builder::paginate(), which calls the raw builder's get() + * internally and never runs eager loading — silently dropping any + * with() that was chained before it, and reintroducing N+1 the + * moment the relation is accessed on the paginated page. + * + * @param int $perPage + * @param int $page + * @return object{total:int,per_page:int,current_page:int,last_page:int,from:int,to:int,data:Collection} + */ + public function paginate(int $perPage = 15, int $page = 1): object + { + $page = $this->builder->paginate($perPage, $page); + + $page->data = $page->data->isEmpty() + ? $page->data + : $this->modelClass::eagerLoad($page->data, $this->withs); + + return $page; + } + /** * Add more relations to eager-load. * @@ -120,4 +145,4 @@ public function __call(string $method, array $args): mixed return $result; } -} +} \ No newline at end of file From 824f937c0404f432cfe8411083c627ef22a05e8d Mon Sep 17 00:00:00 2001 From: benkhalife Date: Tue, 30 Jun 2026 15:43:11 +0330 Subject: [PATCH 2/2] test(relations): verify paginate() eager-loads relations via with() Adds coverage for Model::with(...)->paginate(...) and the select()->with()->paginate() chain, asserting the relation is marked loaded on each row of the page. Also adds a query-count test proving paginate() + with() now uses a constant 3 queries (count + page select + one whereIn for the relation) on a 10-row page, instead of 3 + N from the silent fallback to lazy loading. --- tests/Integration/RelationsTest.php | 74 +++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/Integration/RelationsTest.php b/tests/Integration/RelationsTest.php index 1fe8023..fd9a5eb 100644 --- a/tests/Integration/RelationsTest.php +++ b/tests/Integration/RelationsTest.php @@ -411,6 +411,80 @@ public function test_eager_loading_belongs_to_avoids_n_plus_1(): void $this->assertSame(2, count(DB::getQueryLog())); } + + // ----------------------------------------------------------------------- + // paginate() honours with() + // + // EagerBuilder previously only implemented get() and first() as + // terminal methods that trigger Model::eagerLoad(). paginate() was + // not declared on EagerBuilder, so it fell through __call() straight + // to the underlying Query\Builder::paginate(), which calls its own + // get() internally — never touching eager loading at all. + // + // The practical effect was: with() silently did nothing when + // paginate() was used. Accessing the relation afterwards fell back + // to lazy loading per row, reproducing the exact N+1 problem with() + // exists to prevent — with no error or warning to signal it. + // + // EagerBuilder::paginate() now overrides the forwarded call, runs the + // page query, then eager-loads relations on $page->data before + // returning. These tests verify that fix. + // ----------------------------------------------------------------------- + + public function test_paginate_after_with_eager_loads_relation(): void + { + $this->seedManyUsersWithPosts(userCount: 5); + + $result = RelUser::with('posts')->paginate(5, 1); + $first = $result->data->first(); + + $this->assertTrue( + $first->relationLoaded('posts'), + 'paginate() must honour with() and eager-load relations on the current page.', + ); + $this->assertInstanceOf(Collection::class, $first->posts); + } + + public function test_paginate_after_select_with_eager_loads_relation(): void + { + // Same fix verified via the select()->with()->paginate() chain. + $this->seedManyUsersWithPosts(userCount: 5); + + $result = RelUser::select('id', 'name')->with('posts')->paginate(5, 1); + $first = $result->data->first(); + + $this->assertTrue( + $first->relationLoaded('posts'), + 'paginate() must honour with() through the select()->with()->paginate() chain too.', + ); + } + + public function test_paginate_after_with_avoids_n_plus_1_on_access(): void + { + // The real-world payoff: a paginated list view that does + // Model::with('relation')->paginate(...) and then iterates the + // page printing the relation must NOT trigger N+1 — the relation + // should already be loaded from the single whereIn() query. + $userCount = $this->seedManyUsersWithPosts(userCount: 10, postsPerUser: 1); + + DB::flushQueryLog(); + DB::enableQueryLog(); + + $result = RelUser::with('posts')->paginate($userCount, 1); + foreach ($result->data as $user) { + $user->posts->count(); // must be a no-op — already eager-loaded + } + + $queryCount = count(DB::getQueryLog()); + + // 1 count() + 1 page select + 1 whereIn() for posts = 3, regardless + // of how many users are on the page. + $this->assertSame( + 3, + $queryCount, + 'paginate() + with() must use a constant 3 queries, not one extra query per row.', + ); + } } // -----------------------------------------------------------------------