Skip to content

fix: reuse the same page for every call forwarded by On - #272

Open
tiagoabsantos wants to merge 1 commit into
pestphp:5.xfrom
tiagoabsantos:fix/on-call-reuses-page
Open

tiagoabsantos wants to merge 1 commit into
pestphp:5.xfrom
tiagoabsantos:fix/on-call-reuses-page

Conversation

@tiagoabsantos

Copy link
Copy Markdown

What:

  • Bug Fix
  • New Feature

Description:

On::__call() created a new PendingAwaitablePage on every call, so $page = visit('/')->on()->mobile() opened a new browser context for each separate $page->... call. A click() changed one page and the next assertion ran on a fresh one. On now creates the pending page once and forwards every call to it, like PendingAwaitablePage does with its webpage.

$page = visit('/')->on()->mobile();

$page->click('#toggle');
$page->assertSeeIn('#result', 'Clicked');

tests/Browser/Visit/OnTest.php covers separate calls with and without a device.

Related:

Fixes pestphp/pest#1931

On::__call() created a new PendingAwaitablePage on each call, so `$page = visit('/')->on()->mobile()` opened a new browser context for each separate `$page->...` call and lost the state of the previous action.

Fixes pestphp/pest#1931
@DaniilSkLi

Copy link
Copy Markdown

I hit this bug (pestphp/pest#1931) and opened a PR with a slightly
different approach: #274

Two differences worth discussing:

  1. I kept the class readonly — the lazy ??= initialization of an
    uninitialized (implicitly readonly) property is valid at runtime
    (initialized once from inside class scope), it just needs two targeted
    @phpstan-ignore comments, same as the existing @phpstan-ignore-next-line
    already used in that method.
  2. I added the missing @mixin Webpage|AwaitableWebpage to On, otherwise IDE
    autocompletion for ->on()->...() stays broken due to recursive @mixin
    resolution.

@tiagoabsantos

Copy link
Copy Markdown
Author

On follows PendingAwaitablePage: nullable property, ??=, class not readonly. A one-time write to an uninitialized readonly property is valid PHP. PHPStan still reports property.uninitializedReadonly and property.readOnlyAssignNotInConstructor, so I left the class mutable like PendingAwaitablePage.

The mixin point is right. IDEs that don't follow @mixin PendingAwaitablePage never see Webpage or AwaitableWebpage, so on()->click() has no completion. From has the same annotation. This PR is only the extra browser context on each call. I'll add @mixin Webpage|AwaitableWebpage on On if you want that in here too.

@DaniilSkLi

DaniilSkLi commented Sep 29, 2026 •

Copy link
Copy Markdown

Up to you on scope. If you'd rather keep this PR focused on the context bug, that's fine — the @mixin fix for On is already in #274. Either way works for me.

Side note: while checking this I noticed From has the same @mixin, but it has no __call - the annotation advertises methods that would throw at runtime (Call to undefined method). City methods return PendingAwaitablePage directly, so the mixin is just misleading there and could be dropped in a follow-up.

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.

[Bug]: On::__call() creates a new browser context on every method call - breaks stateful page interactions

2 participants