(feat): allow config values to be changed with hook 'surge/config'

[edwinsiebel]

The values in the config array are hardcoded. Sometimes, it could be useful to change the values.

The hook surge/config (but name can be changed if required), allows values to be changed.

[kovshenin]

Hi @edwinsiebel, thanks for contributing!

Allowing power-users and devs to filter the config has certainly been something on my list. However, the implementation I think is not going to be as simple as applying a filter like in your commit. You see, the config() function is used not only when a request cache is about to be written to disk (at shutdown), but also very very early in advanced-cache.php, at which point none of the plugins or themes have been loaded yet.

So if you add the filter on init or even plugins_loaded, it will only affect cache misses/expires, but not cache hits, causing potential problems. And even in a cache misses the key() function for example uses config() but the result is saved in a static variable, so running it again later will not invoke config() and thus your filters will never run for key() :(

I'm still debating on how to best approach this, but the best option I have so far is actually a wp-config.php directive, which defines a function (or a file perhaps), which Surge can then evaluate. Do you have any thoughts on this?

[edwinsiebel]

Hi @edwinsiebel, thanks for contributing!

Allowing power-users and devs to filter the config has certainly been something on my list. However, the implementation I think is not going to be as simple as applying a filter like in your commit. You see, the config() function is used not only when a request cache is about to be written to disk (at shutdown), but also very very early in advanced-cache.php, at which point none of the plugins or themes have been loaded yet.

So if you add the filter on init or even plugins_loaded, it will only affect cache misses/expires, but not cache hits, causing potential problems. And even in a cache misses the key() function for example uses config() but the result is saved in a static variable, so running it again later will not invoke config() and thus your filters will never run for key() :(

Ah, you're right! I thought as the files where loaded not in a hook/action, these would be changable. However, unfortunately.

I'm still debating on how to best approach this, but the best option I have so far is actually a wp-config.php directive, which defines a function (or a file perhaps), which Surge can then evaluate. Do you have any thoughts on this?

A define would be perfectly legit (http://sandbox.onlinephpfunctions.com/code/3c6bb6a6feae2b72d62ae1794f2cbba5c41bb2d6). You should have a validation process (or proper defaults to fallback to), to determine the correct values.

Having a specific file would be possible, but you have to have a dedicated location, which might be difficult, with different setups or having the file in vcs.

[kovshenin]

Having a specific file would be possible, but you have to have a dedicated location, which might be difficult, with different setups or having the file in vcs.

I meant like

define( 'WP_CACHE_CONFIG', __DIR__ . '/path/to/cache-confg.php' );

We can load that file early, and let them to:

$ttl = 500;
$ignore_cookies = [];

So they can have their own logic too, like if ( $_SERVER['HTTP_USER_AGENT'] == ... ) and so on.

Another option would be a function they can define and I can run if function_exists(), but with this approach I'd have to make sure it's defined early enough, i.e. in wp-config.php but not after wp-settings.php, etc. With a file I guess it's a bit simpler, because most people know where to put their defines.

[edwinsiebel]

Having a specific file would be possible, but you have to have a dedicated location, which might be difficult, with different setups or having the file in vcs.

I meant like

define( 'WP_CACHE_CONFIG', __DIR__ . '/path/to/cache-confg.php' );

yes, this will work as well!

We can load that file early, and let them to:

$ttl = 500;
$ignore_cookies = [];

So they can have their own logic too, like if ( $_SERVER['HTTP_USER_AGENT'] == ... ) and so on.

Personly, I would like to see an array returned, with the values, just like your config. A dev can then still have conditional logic for values, and you can array_merge(_recursive) the values with yours, to have valid settings. Also, validation with an array works (better) than with variables.

Another option would be a function they can define and I can run if function_exists(), but with this approach I'd have to make sure it's defined early enough, i.e. in wp-config.php but not after wp-settings.php, etc. With a file I guess it's a bit simpler, because most people know where to put their defines.

Not my preference, as yours (when reading between the lines).

[edwinsiebel]

Closed in favor in https://github.com/kovshenin/surge/pull/4.