Problem/Motivation

Getting this on 10.1 with PHP 8.1

Fatal error: Declaration of Drupal\jsonapi\Normalizer\JsonapiHypermediaImpostor\JsonapiHypermediaLinkCollectionNormalizer::normalize($link_collection, $format = null, array $context = []) must be compatible with Drupal\jsonapi\Normalizer\LinkCollectionNormalizer::normalize($object, $format = null, array $context = []): ArrayObject|array|string|int|float|bool|null in /data/app/modules/jsonapi_hypermedia/src/Normalizer/JsonapiHypermediaImpostor/JsonapiHypermediaLinkCollectionNormalizer.php on line 59

Basically the return type is missing.

Pretty sure this would mean a new major version is required for D10, and also the D10 status of the module might not be what it seems/is advertised

This blocks #3288143: Automated Drupal 10 compatibility fixes

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 3325083-2.patch1.24 KBlarowlan
#7 3325083-interdiff.txt306 byteslarowlan
#2 3325083.patch965 byteslarowlan
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Status: Active » Needs review
StatusFileSize
new965 bytes
larowlan’s picture

Title: Fatal error: Declaration of Drupal\jsonapi\Normalizer\JsonapiHypermediaImpostor\JsonapiHypermediaLinkCollectionNormalizer::normalize($link_collection, $format = null, array $context = []) must be compatible with Drupal\jsonapi\Normalizer\LinkCollectionNor » Fatal error: Declaration of Drupal\jsonapi\Normalizer\JsonapiHypermediaImpostor\JsonapiHypermediaLinkCollectionNormalizer::normalize($link_collection, $format = null, array $context = []) must be compatible with parent
larowlan’s picture

I think the test results above confirm we need a new major and that the current 'd10 ready' status of the module is a mirage

phenaproxima’s picture

I agree that this module is not D10 ready. Not sure how this bug got past me; maybe upstream changes in core occurred since August which broke it. I'm not really an active maintainer of this project.

That said, I don't know if we need to roll a new major for this. But we definitely do need to drop support for PHP 7 entirely, since union types were introduced in PHP 8.

While we're at it, we'd be well-advised to drop support for Drupal 9.2 and older, and ensure that this works on Drupal 9.3+ as well. I believe that it will, because PHP's covariance rules should allow us to define a more specific return type than the parent method.

phenaproxima’s picture

Status: Needs review » Needs work

I added some test runs to confirm that this patch works on Drupal 9.4 and 9.5 with PHP 8.0 and 8.1. So really, we need to drop PHP 7 support. @larowlan, can you make that change in the patch?

larowlan’s picture

Status: Needs work » Needs review
StatusFileSize
new306 bytes
new1.24 KB

Sure, here you go 🎁

vipin.mittal18 made their first commit to this issue’s fork.

vipin.mittal18’s picture

It is better not to provide a drupal/core version requirement in composer.json because Drupal's composer facade will generate the appropriate metadata based on the info.yml file.

Mr !7 is suffice to fix this issue.

larowlan’s picture

Please ignore the MR as it goes against what the maintainer asked for in 6

Please remove credit for that too.

  • phenaproxima committed bfec4ca on 8.x-1.x
    Issue #3325083 by larowlan: Fatal error: Declaration of Drupal\jsonapi\...
phenaproxima’s picture

Status: Needs review » Fixed

Sorry, @vipin.mittal18, but I agree with @larowlan here; in this case, the merge request did not really add anything to the issue. Adding the PHP requirement is absolutely necessary to fix this properly, because the return type hint is a union type, which is a parse error in PHP 7.

Committed the patch from #7 and pushed to 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.