Skip to content

Add PHP 8.4 and PHP 8.5 compatibility#26

Merged
rhoerr merged 4 commits into
mage-os:mainfrom
SamueleMartini:php85-compatibility
May 12, 2026
Merged

Add PHP 8.4 and PHP 8.5 compatibility#26
rhoerr merged 4 commits into
mage-os:mainfrom
SamueleMartini:php85-compatibility

Conversation

@SamueleMartini

@SamueleMartini SamueleMartini commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds PHP 8.4 and PHP 8.5 compatibility to MageOS_ThemeOptimization while keeping the module working on PHP 8.1/8.2/8.3.

Current last tag: 2.2.0. Suggested next tag: 2.3.0 (backwards-compatible additions/changes only).

cc @rhoerr

Changes

  • Added declare(strict_types=1) to every PHP class in the module (previously missing on ViewTransitions, SpeculationRules, UpdateSpeculationRulesConfigPathPatch).
  • Added explicit return types to every method, including Setup\Patch\Data\UpdateSpeculationRulesConfigPathPatch::apply(), getAliases() and getDependencies(), so subclasses and static analysis don't fall back on the implicit-nullable/void deprecations introduced in PHP 8.4.
  • Typed class constants (const string, const array) on ViewTransitions, SpeculationRules, BfCache and the Http plugin to take advantage of PHP 8.3+ typed constants.
  • Typed nullable parameters explicitly (e.g. int|string|null $store in Http::getConfig()) so PHP 8.4's "implicit nullable parameter is deprecated" warning doesn't trigger.
  • afterSetNoCacheHeaders now uses mixed $result / mixed return type instead of an untyped parameter.
  • Replaced strpos($theme->getCode(), 'Hyva/') === 0 with str_starts_with() in SpeculationRules::isHyva().
  • Constructor-promoted properties are now protected (instead of private) on every class, and private helpers (isHyva, isRequestCacheable, isRequestInExcludePatterns, parseExcludePatterns, isEnabled, getConfig) are also protected, so downstream modules/themes can extend the behavior without plugins.
  • Minor refactors: array_map in place of manual foreach-and-append loops in SpeculationRules::getExcludedExtensions() / getExcludedSelectors(); unset($pattern) after the foreach-by-reference loop in getExcludedPaths(); re-imported Magento\Framework\App\Response\Http as ResponseHttp in the plugin so the fully-qualified-name isn't repeated in method signatures.
  • composer.json: added an explicit php constraint ~8.1.0 || ~8.2.0 || ~8.3.0 || ~8.4.0 || ~8.5.0.
  • Added a CHANGELOG.md with a 2.3.0 — 2026-04-21 entry.

Backwards compatibility

  • No public method signature was changed; only return types were added where previously missing (always matching the documented @return).
  • DataPatchInterface::apply() doesn't declare a return type in Magento framework (its PHPDoc says @return $this); the concrete apply() now declares : self and return $this, which is covariance-compatible with the interface.
  • Visibility changes are widening only (private -> protected), so no downstream break.

Test plan

  • composer install and module registration on PHP 8.1, 8.3, 8.4 and 8.5.
  • Run the included unit tests (vendor/bin/phpunit Test/Unit) on PHP 8.4 and 8.5.
  • Smoke test in a Magento 2.4.7+ storefront: speculation rules JSON is emitted, view-transitions CSS/JS is rendered, bfcache plugin flips no-store on cacheable responses.
  • Verify no deprecation notices are raised with error_reporting(E_ALL) on PHP 8.4 / 8.5.

Suggested release

Tag 2.3.0 (minor bump — only additive/internal changes, no public API break).

@SamueleMartini SamueleMartini requested review from a team and rhoerr as code owners April 21, 2026 17:06
Comment thread Plugin/Framework/App/Response/Http.php Outdated
Comment thread Plugin/Framework/App/Response/Http.php
Comment thread ViewModel/SpeculationRules.php Outdated
Comment thread ViewModel/SpeculationRules.php

@marcelmtz marcelmtz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I didn't find anything critical but the version conflict with the const tyep declared, once we take a decision with that, we can proceed

Comment thread Plugin/Framework/App/Response/Http.php Outdated
rhoerr and others added 3 commits May 11, 2026 21:12
- Drop typed class constants (`const string`/`const array`) for PHP 8.1/8.2
  compatibility per composer.json constraint.
- Restore docblock and inline comments removed across Http, BfCache,
  SpeculationRules, and UpdateSpeculationRulesConfigPathPatch.
- Revert the array_map refactors in SpeculationRules::getExcludedExtensions()
  and getExcludedSelectors() to the original foreach style for readability.
- Update CHANGELOG to drop the typed-const note.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace `@dataProvider` PHPDoc annotations with `#[DataProvider]`
  attributes (PHPUnit 12 dropped the doctype form, which caused
  testIsEnabled and testGetEagerness to be invoked with no arguments).
- Make the two data-provider methods static (PHPUnit 12 requirement).
- Type the variant `$configValue` parameter as `mixed`.
- Migrate `phpunit.xml.dist` to the current schema (drops `verbose`
  attribute, adds `cacheDirectory`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updated release date for version 2.3.0 and added new features and improvements.
@rhoerr rhoerr dismissed marcelmtz’s stale review May 12, 2026 01:26

Handled feedback

@rhoerr rhoerr merged commit 36234fd into mage-os:main May 12, 2026
4 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