Herd Pro not compatible with Kirby because of dump() helper

[lukasbestle]

I'll bring that up again because Kirby doesn't work with Herd Pro (or it doesn't if you want to use the Dump feature, which is a main argument to use the Pro version) and since there is an official video on how to use Herd with Kirby, that sounds like a reason to discuss the issue again.

https://www.youtube.com/watch?v=Mb_-bo77IPc

Originally posted by @sschlein in #4462 (comment)

[lukasbestle]

What we could do is to detect if the existing dump() function was defined within a file in the Herd folder. Then we can reliably say that the user intends to use Herd's dump feature and disable our function even without defining the constant.

[bastianallgeier]

@lukasbestle I like that idea.

[andielliott]

I'd actually raised this with Beyond Code first, and they replied with:

It seems like Kirby does not properly check if a global function is already declared.
Our custom dump-loader defines this function and therefore it should overload Kirby's function.

This line should have a if (!function_exists('dump')) around it...

kirby/config/helpers.php

Line 117 in a537a4d

function dump(mixed $variable, bool $echo = true): string

[lukasbestle]

So far we didn’t do that on purpose because it causes unexpected behavior. For most cases this would still be true, but with Herd and other server setups that externally define the helper, it is expected that the external helper is used.

[distantnative]

Maybe let's take a news perspective her: We always didn't what to do if (!function_exists()) as it could lead to unwanted consequences with global helpers like e.g. go() if any other than the Kirby one is used unknowingly.

But since dump() is solely used for debugging.... maybe that is a special one where we could change our policy?

[bastianallgeier]

@distantnative I thought along those lines as well. Other helpers could indeed be super destructive, but I've never seen a dump method used for anything else than dumping. I think it's a pretty universal standard in PHP land. So I would say that we could really make an exception here.

[sschlein]

While I have absolutely no idea how Kirby works and have only used it for testing Herd, Herd Pro and Tinkerwell and run into the dump problem with these tools, I'd agree with that.

Most major frameworks and systems are using the function_exists for dump() so that it can be set differently and they seem to do well. I absoutely agree with things that are relevant to the application itself and that shouldn't be overwritten easily.

[lukasbestle]

I agree. So far we only ever had this issue with dump(). We added the disable constants to all helpers and I think they can still make sense e.g. if someone wants to customize/replace the helpers in a plugin. But for dump() it is indeed OK if Kirby accepts an externally defined function by default. So here we could have a check that accepts either the constant or function_exists().