Skip to content

Fix issue with Browser tests#54

Merged
ganyicz merged 5 commits intolivewire:mainfrom
WendellAdriel:fix/browser-test-issues
Feb 25, 2026
Merged

Fix issue with Browser tests#54
ganyicz merged 5 commits intolivewire:mainfrom
WendellAdriel:fix/browser-test-issues

Conversation

@WendellAdriel
Copy link
Contributor

@WendellAdriel WendellAdriel commented Feb 24, 2026

Issue

`BlazeRuntime` is a singleton shared across all views via `View::share('__blaze', ...)`. Blaze-compiled components (like `flux:error`) access `$errors` through `$__blaze->errors`, which triggers the `__get` magic method.

The original code was:
```php
return $this->errors ??= $this->env->shared('errors') ?? new ViewErrorBag;
```

The `??=` operator assigned the `$errors` value to a protected property on first access, then returned the cached value on all subsequent accesses. Since `BlazeRuntime` is a singleton, this meant:

  1. Request Cleans up styles #1 (GET /login) — `$errors` is loaded as empty `ViewErrorBag` and cached on the singleton via `??=`
  2. Request Adds @once directive as a validation check & extra test cases #2 (POST /login fails, redirect back with errors) — `ShareErrorsFromSession` middleware shares fresh errors into the view factory, but `$__blaze->errors` still returns the stale empty cached value
  3. The `flux:error` component never sees the validation errors, so error messages are never displayed

This only manifests in long-lived processes (Pest Browser's in-process HTTP server, Laravel Octane, etc.) because in standard PHP-FPM each request gets a fresh process and singleton.

Fix

Remove the `??=` caching entirely — always read fresh from the view factory via `$this->env->shared('errors')`. Also removed the `protected ViewErrorBag $errors` property since it's no longer used.

The `shared()` call costs ~135ns per component, which is negligible since each component reads `$errors` exactly once (Wrapper.php stores it in a local variable). This approach fixes all long-lived environments without relying on lifecycle hooks.

How to replicate issue

Using the Laravel Livewire starter kit, create browser tests that first successfully login, then attempt a failed login. The second test never sees validation errors because the singleton cached an empty error bag from the first request.

@ganyicz
Copy link
Collaborator

ganyicz commented Feb 24, 2026

Hey @WendellAdriel, thanks!

I'm pretty sure I added this for performance reasons, let me run some benchmarks and see if we can safely drop it.

Either way, if the issue is that blaze runtime retains stale data wouldn't a more direct solution be to make the runtime scoped? Or flush the data when request finishes?

@WendellAdriel
Copy link
Contributor Author

@ganyicz good point.
Let me test these options to check if I can update the PR, thanks!

@WendellAdriel
Copy link
Contributor Author

Can you take a look in this new approach @ganyicz ?

calebporzio and others added 3 commits February 24, 2026 20:40
Remove ??= caching on $errors — always read fresh from the view factory.
The previous approach (resetting via terminating callback) didn't cover
all long-lived environments. Reading fresh costs ~135ns per component,
which is negligible since each component reads $errors exactly once.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@calebporzio
Copy link
Contributor

K, thought the approach and had a bunch of Claude instances try some different approaches and investigate and came up with this: (which I agree with and think we should go with)

Thanks for the bug report and fix — the diagnosis is spot on. The ??= on the singleton is definitely the root cause.

I went a slightly different direction on the fix though. Instead of caching $errors on the singleton and resetting it via $app->terminating(), I removed the caching entirely — __get now reads fresh from the view factory on every access.

Here's why:

terminating() doesn't fire everywhere. It fires during the full HTTP kernel lifecycle, but not in unit tests, Pest Browser's in-process server, or other non-standard long-lived environments. If the hook doesn't fire, the bug silently returns. Removing the cache entirely means there's nothing to reset — it works in every environment with zero lifecycle awareness.

Performance is fine. I considered whether caching matters since we sometimes optimize pages with 25k+ components. Benchmarked it (PHP 8.4, 1M iterations):

Approach ns/call At 25k components
??= cached property (original, stale) 6.6ns 0.17ms
$this->env->getShared()['errors'] 23.7ns 0.59ms
$this->env->shared('errors') via Arr::get 135.5ns 3.39ms

Using getShared() (raw array return) instead of shared() (which goes through Arr::get dot-notation parsing) keeps it at ~24ns per call. At 25k components that's 0.59ms total — and each component only reads $__blaze->errors once (Wrapper.php stores it in a local $errors variable), so it's exactly 1 call per component per request.

I also considered using a scoped binding ($this->app->scoped(BlazeRuntime::class)) which felt like the cleanest lifecycle approach. But it has a fatal flaw: View::share('__blaze', $instance) stores a direct object reference. When Octane calls forgetScopedInstances(), the container creates a new BlazeRuntime, but the view factory still holds the old one — templates keep using the stale object. And even if you worked around that (re-share via middleware), scoped would nuke the $paths, $compiled, and $blazed filesystem caches that BlazeRuntime deliberately keeps across requests. At 25k components, rebuilding those caches every request would be a significant regression.

Net change: removed ??= caching, removed $errors property, removed the requestTerminated() method and terminating() callback. One-line behavior change in __get, everything else is cleanup.

@calebporzio
Copy link
Contributor

If you're totally against even the tiniest amount of performance loss here, we could just go with terminating or another listener along those lines. But I say we just go with it. Thanks @WendellAdriel

@WendellAdriel
Copy link
Contributor Author

Thanks @calebporzio! 💪

My first approach was to remove the cache as well, but after talking with @ganyicz we were a little concerned about performance, if you feel it's ok, I'm ok on removing the cache.

@ganyicz ganyicz merged commit c855529 into livewire:main Feb 25, 2026
3 checks passed
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