Problem/Motivation

I got 2 fatal errors with PHP 8.2

TypeError: Drupal\conditional_fields\ConditionalFieldsFormHelper::buildJquerySelectorForField(): Argument #1 ($field) must be of type array, null given, called in C:\Users\www\web\modules\contrib\conditional_fields\src\ConditionalFieldsFormHelper.php on line 202 in Drupal\conditional_fields\ConditionalFieldsFormHelper->buildJquerySelectorForField() (line 920 of C:\Users\www\web\modules\contrib\conditional_fields\src\ConditionalFieldsFormHelper.php).

TypeError: Drupal\conditional_fields\ConditionalFieldsFormHelper::elementAddProperty(): Argument #1 ($element) must be of type array, null given, called in C:\Users\www\web\modules\contrib\conditional_fields\src\ConditionalFieldsFormHelper.php on line 197 in Drupal\conditional_fields\ConditionalFieldsFormHelper->elementAddProperty() (line 843 of C:\Users\www\web\modules\contrib\conditional_fields\src\ConditionalFieldsFormHelper.php).

because PHP 8.2 want check type of variable strictement

Steps to reproduce

Proposed resolution

remove array or change to mixed

public function func buildJquerySelectorForField(array $field)
public function elementAddProperty(array $element, $property, $value, $position = 'prepend')

to

public function buildJquerySelectorForField($field) 

public function elementAddProperty($element, $property, $value, $position = 'prepend')
CommentFileSizeAuthor
#3 3423256-3.patch2.82 KBhimanshu_jhaloya
fixed_php82.patch2.61 KBlazzyvn

Issue fork conditional_fields-3423256

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

lazzyvn created an issue. See original summary.

himanshu_jhaloya made their first commit to this issue’s fork.

himanshu_jhaloya’s picture

himanshu_jhaloya commented
Assigned: Unassigned » himanshu_jhaloya
Status: Active » Needs review
StatusFileSize
new 3423256-3.patch2.82 KB

Patch Failed to Apply. Recreate the new patch Please Review.

himanshu_jhaloya’s picture

himanshu_jhaloya commented
Assigned: himanshu_jhaloya » Unassigned
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Version: 4.0.0-alpha5 » 4.x-dev
Priority: Critical » Normal
Status: Needs review » Postponed (maintainer needs more info)

Thanks for the report and the efforts in here!

But first: Both patches are missing comments/notes/details about what has been changed and why it has been fixed this way. Also what has the second patch done differently?

Apart from that: I decreased the issue priority. Descriptions of the Priority and Status values can be found in the Drupal project issues documentation.

And also, please provide patches and MR always against latest dev. Thanks!

lazzyvn’s picture

lazzyvn commented
Priority: Normal » Critical

Hello,
I'm sorry but I don't know how to report all the information you need.
You can check with php 8.2 required for drupal 10
in node form for example, if you can hook alter form you can add ajax event for field and ajax rebuild the form, then no need to return all elements. that means it will have empty elements. So

public function func buildJquerySelectorForField(array $field);
public function elementAddProperty(array $element, $property, $value, $position = 'prepend');

php 8.1 below it will not check the variable type in the method but 8.2 will sort the fatal error. For me it is critcal bug because Drupal 10 require php 8.2
I suggest to change to

public function func buildJquerySelectorForField($field)
public function elementAddProperty($element, $property, $value, $position = 'prepend')

or

public function func buildJquerySelectorForField(mixed $field)
public function elementAddProperty(mixed $element, $property, $value, $position = 'prepend')

I check the patch it works for the latest branch dev. I will make a merge request if you need it

BTW: Conditional Fields module is very interesting, can you add me as maintainer, i can contribute my time to fix all the errors of phpcs, issue report and validate with gitlab-ci

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Priority: Critical » Normal
Status: Postponed (maintainer needs more info) » Needs work

Thanks for coming back and reporting more details! Very much appreciated. Very helpful. And yes a reroll or MR would be very much appreciated! +1

oh and, Sorry but I have to decrease the issue priority. Descriptions of the Priority and Status values can be found in the Drupal project issues documentation.

can you add me as maintainer, i can contribute my time to fix all the errors of phpcs, issue report and validate with gitlab-ci

Thanks for your will to help! We had a co-maintainer request accepted some days ago exactly in the scope of the tasks you describe, thanks, but if we need help, I would love to come back to your offer. +1 Very much appreciated. I have cleaned up the issue queue over the whole weekend with over 200 issues and committed RTBCs and more or less no sleep and plan an upcoming BETA release soon. Stay tuned.

samit.310@gmail.com made their first commit to this issue’s fork.

samit.310@gmail.com opened merge request !45

samitk’s picture

samitk commented
Status: Needs work » Needs review

HI @dqd

Reroll and MR created, Please review.

Thanks
Samit K.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

This should have a rebase now that Gitlab CI testing is enabled.

samitk’s picture

samitk commented
Status: Needs work » Needs review

Hi @heddn,

Rebased with 4.x, Please review.

Thanks
Samit K.

heddn closed merge request !45

heddn reopened merge request !45

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

Unfortunately, I think this needs another rebase. I think the major change here is changing from array => mixed?

andreastkdf made their first commit to this issue’s fork.

andreastkdf’s picture

andreastkdf commented

Just pushed a reroll.

andreastkdf’s picture

andreastkdf commented
Status: Needs work » Needs review
andreastkdf’s picture

andreastkdf commented

fyi this is now rebased and last commit also includes an additional fix - needs review :)

benstallings’s picture

benstallings commented
Status: Needs review » Needs work

I get a merge conflict when I try to rebase on 4.x.

benstallings’s picture

benstallings commented
Status: Needs work » Closed (outdated)

since we're now targeting PHP 8.4, I think this ticket is outdated.

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.