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
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3325083-2.patch | 1.24 KB | larowlan |
| #7 | 3325083-interdiff.txt | 306 bytes | larowlan |
| #2 | 3325083.patch | 965 bytes | larowlan |
Issue fork jsonapi_hypermedia-3325083
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
Comment #2
larowlanComment #3
larowlanComment #4
larowlanI think the test results above confirm we need a new major and that the current 'd10 ready' status of the module is a mirage
Comment #5
phenaproximaI 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.
Comment #6
phenaproximaI 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?
Comment #7
larowlanSure, here you go 🎁
Comment #10
vipin.mittal18It 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.
Comment #11
larowlanPlease ignore the MR as it goes against what the maintainer asked for in 6
Please remove credit for that too.
Comment #13
phenaproximaSorry, @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!