Only write the fbp and fbc cookies when there is a pixel to send to

[loevgaard]

Fixes #28

Problem

StoreFbpSubscriber and StoreFbcSubscriber wrote their cookies on the response to every main request, including in applications with zero pixels configured. Every Set-Cookie header makes a response uncacheable for Symfony HttpCache, Varnish and CDNs, so this quietly cost cacheability sitewide for something that may not be in use at all. Neither subscriber consulted the pixel provider, unlike AddLibraryToTagBagSubscriber, which already checks getPixels().

The private setCookie() method was a predicate whose name read like a command.

Change

• Both subscribers skip when the pixel provider returns no pixels: there is nowhere to send events to, so there is nothing to identify the browser for.
• The cookie names and the 90-day lifetime move into a shared Cookie\Cookies class used by the two contexts that read them and the two subscribers that write them, replacing four magic strings.
• setCookie() is renamed to shouldSetCookie().

Removed after review

An earlier revision also added cookies.fbp / cookies.fbc switches and skipped writing on 4xx/5xx responses. Both are gone. The switches were speculative configuration with no request behind them, and the status-code guard did not achieve what it claimed — a status code says nothing about whether a response is a cacheable HTML page — while Meta's own parameter builder does no such check. The pixel guard is what actually stops cookies on sites that do not use them.

Tests

StoreFbpSubscriberTest (its first): cookie set, not http-only so the browser pixel can read it, skipped without pixels, without consent and on a sub request, and the two-hour renewal window in both directions. StoreFbcSubscriberTest mirrors it, plus the fbclid presence check and the context returning no value.

Verified on PHP 8.1 and 8.4, with --prefer-lowest and highest.

[codecov]

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.95%. Comparing base (b944058) to head (0fb55dd).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/Cookie/Cookies.php	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master      #50      +/-   ##
============================================
+ Coverage     90.49%   97.95%   +7.45%     
- Complexity      148      151       +3     
============================================
  Files            31       32       +1     
  Lines           484      488       +4     
============================================
+ Hits            438      478      +40     
+ Misses           46       10      -36     

Files with missing lines	Coverage Δ
src/Context/Fbc/CookieBasedFbcContext.php	100.00% <100.00%> (ø)
src/Context/Fbp/CookieBasedFbpContext.php	100.00% <100.00%> (ø)
src/EventSubscriber/StoreFbcSubscriber.php	100.00% <100.00%> (+80.00%)	⬆️
src/EventSubscriber/StoreFbpSubscriber.php	100.00% <100.00%> (+84.00%)	⬆️
src/Cookie/Cookies.php	0.00% <0.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[loevgaard]

Not sure we need this?

[loevgaard]

Agreed, and removed. These switches came from my own hedge in the original issue ("consider a config switch") rather than from anything anyone asked for. The case they were meant for — a site where the browser pixel already writes the cookies — does not need them either: the pixel simply overwrites the same cookie name. The pixel guard is what actually matters for sites that don't use cookies at all, and that stays.

The cookies node is now introduced by #51, which only holds domain and lifetime.