Problem/Motivation

TypeError: Drupal\facets\Plugin\facets\facet_source\SearchApiDisplay::getCount(): Return value must be of type string, none returned in Drupal\facets\Plugin\facets\facet_source\SearchApiDisplay->getCount() (line 474 of modules/contrib/facets/src/Plugin/facets/facet_source/SearchApiDisplay.php).

This issue is for 3.x. The 2.x issue is here:
https://www.drupal.org/project/facets/issues/3480004

CommentFileSizeAuthor
#11 255.diff1.58 KBioana apetri
#4 3488396-typeerror-return-value.patch1.4 KBmikell

Issue fork facets-3488396

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:

About issue forks

Comments

csmdgl created an issue. See original summary.

csmdgl opened merge request !255

csmdgl’s picture

csmdgl commented
Status: Active » Needs review
mikell’s picture

mikell commented
StatusFileSize
new 3488396-typeerror-return-value.patch1.4 KB

Thx, can confirm this patch fixed it.

mikell’s picture

mikell commented
Status: Needs review » Reviewed & tested by the community
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

Imho this can be solved without changing the return type at Drupal\facets\Plugin\facets\facet_source\SearchApiDisplay::getCount().

alorenc’s picture

alorenc
Location Wolsztyn, 🇵🇱
commented
Assigned: Unassigned » alorenc
alorenc’s picture

alorenc
Location Wolsztyn, 🇵🇱
commented
Assigned: alorenc » Unassigned
Status: Needs work » Needs review
dxvargas’s picture

dxvargas commented
Status: Needs review » Reviewed & tested by the community

The MR is working.
I'm not sure about removing the return type declaration, "?string" could be there. But since the return type declarations are not consistently used in the class and it's not used in the interface, removing is also an option that makes sense.
I mark the issue as RTBC.

dj1999’s picture

dj1999 commented

The #4 fixed the error.

ioana apetri’s picture

ioana apetri commented
StatusFileSize
new 255.diff1.58 KB

Hello, I added the diff based on the MR. Thanks!

liam morland made their first commit to this issue’s fork.

herved’s picture

herved commented
Status: Reviewed & tested by the community » Needs work

There is a conflict in the MR, recent changes on 3.x affected this and the return type was removed to fix phpstan.
See https://git.drupalcode.org/project/facets/-/commit/28332851b927d94f4deb3...

I think we could either:
- close this
- or add the return type to ::getCount in FacetSourcePluginInterface, FacetSourcePluginBase and SearchApiDisplay, but that would affect backwards compatibility I assume.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review

I re-rolled the MR.

herved’s picture

herved commented

Just a thought, but the issue title is no longer accurate since the PHP TypeError doesn't happen anymore on 3.0.x.

So this now adds return type to SearchApiDisplay::getCount which could be done but this could affect BC and I wonder if we should apply the same to other ::getCount implementations for consistency? as mentioned in #13

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I don't see how adding the return type declaration would cause a problem. Having no return type declaration means it could return anything. The merge request just narrows the possibilities. Adding the return type declaration to the interface could cause a backwards-compatibility issue. Better to add it to the implementations first.

joaopauloscho’s picture

joaopauloscho commented

This is fixed by https://www.drupal.org/project/facets/issues/3502957 included in the 3.0.2.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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