Closed BernhardBaumrock closed 1 day ago
@ryancramerdesign - as a quick initial followup - PW won't even load in PHP8.4 at the moment because of those session errors at the end of the OP above. As a quick hack to get things going so I can start testing, I commented out these lines:
if($this->config->https && $this->config->sessionCookieSecure) {
ini_set('session.cookie_secure', 1); // #1264
if($this->config->sessionNameSecure) {
session_name($this->config->sessionNameSecure);
} else {
session_name($this->config->sessionName . 's');
}
} else {
session_name($this->config->sessionName);
}
Once logged in, I could re-enable them.
The "Implicitly marking parameter $sessionHandler as nullable" is easily fixed by adding a ?
mark, eg:
public function sessionHandler(?WireSessionHandler $sessionHandler = null) {
Deprecated: str_getcsv(): the $escape parameter must be provided as its default value will change in wire/core/Sanitizer.php:4506
Deprecated: ProcessWire\SelectableOptionManager::updateLanguages(): Implicitly marking parameter $event as nullable is deprecated, the explicit nullable type must be used instead in Fieldtype/FieldtypeOptions/SelectableOptionManager.php on line 785
There are going to be a LOT of these nullable errors popping up so note that in some cases you might want to define multiple types, like string|array|null
rather than one type with the preceding ?
Sorry to spam, but just wanted to note that I've updated Tracy to deal with the most obvious of its 8.4 deprecation errors - it had lots that were making things a mess and in one case causing a cannot modify header information error.
More implicit null errors:
Database.php on line 142
MarkupFieldtype.php on line 82
MarkupQA.php on line 74
MarkupQA.php on line 620
Pagefile.php on line 428
Pageimage.php on line 1790
Just a reminder that the ?
prefix for type declarations isn't available until PHP 7.1 and union/multiple types aren't available until 8.0, so you might need an alternate approach or change the requirements listed here: https://processwire.com/docs/start/install/new/
@adrianbj @BernhardBaumrock
There are a lot of cases where an argument needs to allow any type of value, and the function determines the action based on the type. The nullable "?" option requires a specific declared type, and PHP won't accept "mixed?" as the type because it says "mixed already includes null". Naming all types (PHP8) isn't always possible. And PHP also won't accept function foo(?$val = null)
, so I seem to be stuck. Maybe I'll just focus on the instances where "?" can be used, and then later revisit the instances where it can't. For the moment though, it seems we'd have to suggest that people avoid using PW with PHP 8.4.
It sounds like it'll be a tricky update, as null is what we use universally to identify that an argument value wasn't provided. PhpStorm finds 476 instances just in the core, and there's no one-fits-all solution. There really is no good substitute for null, but maybe \0
null string is the closest we can get. There are cases where we could replace it with false, blank string, or that null byte string, but might break anything that implements ProcessWire interfaces (likes modules), regardless of PHP version, because the argument definition has to change to accommodate this change in PHP. I'm not sure what the folks at PHP were thinking with this particular update. In any case, thanks for letting me know. I think I might have to get a new main/master version out and then come back to this. But the fact that you got Tracy working with it already gives me hope that maybe it won't be as difficult as it sounds.
Hey @ryancramerdesign - if it needs to allow any type, then you can simply leave off the type declaration entirely and then 8.4 doesn't care if you assign a default of null without the ?
, eg $var = null
.
Remember that you can also limit the types like string|array|null $var = null
and that will also be OK.
I think in some cases false
or an empty string might actually be more semantically correct, but will probably require a lot more care and testing to implement.
@adrianbj so it's just those that already have a type declared in the arguments? Like (array $a = null) or (PageArray $products = null), etc? And an undeclared type argument like ($var = null) is okay? If so, that sounds much more manageable. Also leaves open the option of just dropping the type from the argument in some rare cases, as it's already declared in the phpdoc.
On Thu, Nov 28, 2024, 11:23 AM Adrian Jones @.***> wrote:
Hey @ryancramerdesign https://github.com/ryancramerdesign - if it needs to allow any type, then you can simply leave off the type declaration entirely and then 8.4 doesn't care if you assign a default of null without the ?, eg $var = null.
Remember that you can also limit the types like string|array|null $var = null and that will also be OK.
I think in some cases false or an empty string might actually be more semantically correct, but will probably require a lot more care and testing to implement.
— Reply to this email directly, view it on GitHub https://github.com/processwire/processwire-issues/issues/2000#issuecomment-2506464992, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACQEUCXHSGWNU4HEZZQNCL2C47PFAVCNFSM6AAAAABSMMVXKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBWGQ3DIOJZGI . You are receiving this because you were mentioned.Message ID: @.***>
Yes @ryancramerdesign - that's my experience so far. And in actuality when trying to support PHP 7, you need to be careful because union types aren't available until 8, so the simplest thing in most cases seems to be to remove the type declaration (if you need to define the default as null), or change the default from null to an empty string or array.
Thanks @BernhardBaumrock @adrianbj I've updated all the files that I could find that needed adjustments for this. I'm not yet running PHP 8.4 myself, please let me know if you spot any others.
Thanks @ryancramerdesign - I'll test here and get back to you with any further issues. Please note though that the ?
required PHP7.1 so you'll need to adjust the requirements - currently it just states "7".
So far, everything looks good - thanks for the quick turnaround on this.
Hey @ryancramerdesign I just tried and had no issues on my quick testrun. Thx!