Viewing 5 replies - 1 through 5 (of 5 total)
  • Plugin Author Frank Goossens

    (@futtta)

    I had considered removing those constants altogether (as I’m not sure where/ when this is used, I thought this was mainly/ only for wp-admin purposes) but decided on keeping them after all, because I *think* the intention of Turl (the original author) was to ensure other plugins would not try to re-compress what is already compressed.

    Now if we assume that indeed was the intention, then leaving e.g. COMPRESS_SCRIPTS to true (as per your test), could result in plugins trying to aggregate or compress what already has been aggregated/ compressed. So although I’ve yet to see such an edge case, I’m not convinced this would be a good idea.

    But as you came up with this, you might have a use case where this would make sense? Can you elaborate a bit?

    frank

    Thread Starter Madalin Ignisca

    (@madalinignisca)

    I think that it would be a best practice not to set Constants that may impact other plugins or custom installations.

    Usually, and this is not specific to WordPress only, Constants should only be set by the core and in a config file, in WordPress in wp-config.php.

    Other constants that would be related only to specific plugins/themes, could be used, but offer a way to change them in case that the user needs to do so, so again, a test if it is set should be done. The value of a constant should offer first the possibility to be set through System Variables.

    Best would be to start with a check on environment:

    if($constant = getenv('MY_AWESOME_CONSTANT') && getenv('MY_AWESOME_CONSTANT') !== 'my_awesome_value' && ! defined('MY_AWESOME_CONSTANT))
    {
      define('MY_AWESOME_CONSTANT', 'my_awesome_value');
    }
    else
    {
      add_action( 'admin_notices', function() {
        echo '<div class="warning">Plugin X can't work with <strong>MY_AWESOME_CONSTANT</strong> set to a different value then <em>my_awesome_value</em><br />Please refer to <a href="https://address_to_issue">KB_X</a> to find a solution</div>';
      });
    }

    If the functionality depends on an exact value of a Constant, then instead of throwing an error or work strange, it should offer a way to inform the user the situation and offer a way of solutioning it.

    Setting of Constants common as usage it is not a best practice when delivering modules like WordPress plugins.

    In my case, I’m working on a SaaS application that offers WordPress as a service, and all possible WordPress Core Constants and many other settings are controlled by the management application responsible of deploying and managing all WordPress installations.

    In this case of the Caching and Css/Js assets management, those Constants are set depending on certain situations the users are using their websites (for example for Admins and Roles that are set for testing purposes.

    For the production instances, I am setting those Constants to true because of some other custom integration I have done, so I had to do the tests in your plugin, so I don’t get megabytes of logs full of the warnings ??

    I came up on your plugin, as it is doing it’s job better then any other plugins we have tested.

    Keep up the good work with it!

    Regards,
    Madalin

    Plugin Author Frank Goossens

    (@futtta)

    right, redefining constants results in warnings :facepalm:

    committed this to the ever closer by AO 2.0 here; https://github.com/futtta/autoptimize/commit/9f90e0b61c9cd5b5849bcd4ec27e3c48809f53fd

    thanks for the feedback!
    frank

    Thread Starter Madalin Ignisca

    (@madalinignisca)

    Good to know you’re using Github. I’ll bug with PR from now on…

    Plugin Author Frank Goossens

    (@futtta)

    while on github, do check out the in-development version of what will soon become AO 2.0 (download here). there are some big improvements (on average 30% faster creation of autoptimized files) & new functionality (option to enable/ disable aggregation of inline CSS/ JS, option to remove google fonts in CSS will go in tomorrow, …) in there. (the commit history has all the details) ??

    frank

Viewing 5 replies - 1 through 5 (of 5 total)
  • The topic ‘test for constants’ is closed to new replies.