search

LM-Commons / LmcUser

A generic user registration and authentication module for Laminas. Supports Laminas\Db and Doctrine2. (Formerly ZfcUser)
BSD 3-Clause "New" or "Revised" License
15 stars 16 forks source link

PHP 8.2 #41

Closed lon9man closed 1 year ago

lon9man commented 1 year ago

Hello

WHAT I DID tried to test project on PHP 8.2

WHAT I GET Uncaught ErrorException: Creation of dynamic property LmcUser\Authentication\Adapter\AdapterChainEvent::$request is deprecated in /home/xxxxx/app/vendor/lm-commons/lmc-user/src/LmcUser/Authentication/Adapter/AdapterChainEvent.php:100

WHAT I NEED absence of the errors on PHP 8.2

thanks

curzio-della-santa commented 1 year ago

Agree with lon9man

I've checked the code, the mentioned line sets a dynamic-property that does not seem to be used anywhere.

Since there is a public getter for Request, this line could be removed without causing problems to the vast majority of user. If someone is relying on this property to exist (despite the poor programming), can change the code to use the getter instead.

Because depreaction notices are usually not shown in production environments, is it currently for me only an annoying problem in test environment.

tomekszad commented 1 year ago

@lon9man @curzio-della-santa did you manage to overcome this issue? If yes, how? Thank you for your inputs!

curzio-della-santa commented 1 year ago

@tomekszad I've now written the PR to delete this malicious line, hope the maintainers of the library approve it for next release.

As said above, is only a warning for a line that can be simply commented out or removed in development environment, where deprecation warning are normally returned in the output.

In my current situation I can simply comment out the line after composer install.

Can be done with following command on CLI:

sed -i '100 s/^/\/\//' vendor/lm-commons/lmc-user/src/Authentication/Adapter/AdapterChainEvent.php

(Warning, this script comments out line 100 regardless what's in it, use at own risk)

tomekszad commented 1 year ago

@curzio-della-santa thank you for your reply! We on the other hand, have simply forked the repo, and fixed it on our fork.

Cheers!

visto9259 commented 1 year ago

Hi everyone,

Let me take a look at the PR and make a release.

Sorry for the delay, we are only spending time on maintenance on our spare time...

lon9man commented 1 year ago

@curzio-della-santa did you test other sources how it works with PHP 8.2? any other issues?

visto9259 commented 1 year ago

This will take a few iterations.
There are many deprecation notices when running the test suite on php 8.2 in addition to the one mentioned above.

curzio-della-santa commented 1 year ago

@curzio-della-santa did you test other sources how it works with PHP 8.2? any other issues?

Yes I did test, and I found no additional dynamic assignment in this library.