Adding 'fragment_cache_init'?

[gmazzap]

I was thinking about extend fragment cache with a couple of custom, site-specific, handlers. There is no documentation for this task but looking at code is pretty clear that to add additional handlers external code should rely on global $fragment_cache variable.

Leaving aside discussion about goodness of global variables, what I see is that there is no way to call $fragment_cache->add_fragment_handler on global $fragment_cache object before $fragment_cache->run(); is called.

What I propose is to add an hook, maybe 'fragment_cache_init' after global $fragment_cache object is created and default handlers have been added.

I forked and branched the repo to show with code what I mean: see https://github.com/Giuseppe-Mazzapica/fragment-cache/compare/fragment_cache_init-hook?expand=1

The benefits should be pretty clear: custom handlers can be added using that hook, something like:

add_action( 'fragment_cache_init', function( $fragment_cache ) {
  $fragment_cache->add_fragment_handler( 'gm-foo-bar', 'GM\\Foo\\Bar' );
} );

Side effect is that same hook can be used to disable specific handler initialization before it is enabled:

add_action( 'fragment_cache_init', function( $fragment_cache ) {
  unset( $fragment_cache['menu'] );
} );

Sure that hook can be used to do anything, ruin plugin integrity included, but actually global variable is not safer.

[Rarst]

There is no documentation for this task

Subtle. ;)

Leaving aside discussion about goodness of global variables

It's interesting how WP takes pride in zillion of plugins, but has zero mechanisms or conventions for them to interact. Is global a good thing? In general no. In WordPress it's not like there is clean and pretty way to do what it does without some singleton abomination. So could as well...

before $fragment_cache->run(); is called

I get that you consider this an issue, but you kind of omit why is this an issue? Neither handlers approach was designed with any regard to run() timing or the method executing anything but hooks setup.

The benefits should be pretty clear: custom handlers can be added using that hook

One more way to do something already possible isn't necessarily benefit by default. You can use init for this just as well and you don't have to learn/remember/debug custom plugin–specific hook,

can be used to disable specific handler initialization before it is enabled

So can init be, same as above.

My conclusions on suggestion are roughly following:

1. As addition to global this is a little of addition for the sake of addition. I had put a lot of effort into having very minimal code/API for the plugin and having it refined to the main function. It does only what it has to do.
2. As replacement for the global this is backwards compatibility break (so no earlier than 2.0 because semver) and (in current form) it narrows timing of access to the instance to the load, which is inconvenient and (in this regard) inferior to global.

[gmazzap]

It's interesting how WP takes pride in zillion of plugins, but has zero mechanisms or conventions for them to interact. Is global a good thing? In general no. In WordPress it's not like there is clean and pretty way to do what it does without some singleton abomination. So could as well...

I said "leaving aside" because really I know that in WordPress context that discussion make almost no sense: avoid global in plugins can't be a so great enhancement if WP itself has zillion of global vars. In addition I know that you can't remove the global, because actually is the only method to make plugin container accessible.

I get that you consider this an issue, but you kind of omit why is this an issue?

I don't think it is an issue, but I think my approach simplify the process.

If I'm not wrong (and it's reasonable that I am) the code needed is:

add_action( 'after_setup_theme', function() { // run before 'init', so we are in time
  if ( ! class_exists('Rarst\Fragment_Cache\Plugin' ) ) {
    return; // must check plugin is active
  }
  global $fragment_cache;
  $fragment_cache->add_fragment_handler( 'gm-foo-bar', 'GM\\Foo\\Bar' );
} );

Using my approach code should be:

add_action( 'fragment_cache_init', function( $fragment_cache ) {
  $fragment_cache->add_fragment_handler( 'gm-foo-bar', 'GM\\Foo\\Bar' );
} );

As said is not an issue, but make life easier for external developers and I bet it's easier to document, when and if you'll decide to do it ;P

That said, feel free to close the issue, I'm already working on the custom handler and I have to make it work that is far harder than add it to your plugin. And, I realised to didn't said that: thanks for it, it's really a good one.

[Rarst]

In addition I know that you can't remove the global, because actually is the only method to make plugin container accessible.

Actually passing instance through the filter is one of the ways to do without global. But in that case can only access it via that filter so only for anti-global purists mostly (cough toscho cough).

Using my approach code should be:

So the difference is basically the need to check if plugin exists. This is valid for developer experience and exactly what I was trying to get you to explain. :)

I guess I am not adding it right now, but I am keeping this in mind (for v2 if it makes sense to me at that point).

thanks for it, it's really a good one

You are very welcome. :) Keep me posted about how it works out for you.

[gmazzap]

Actually passing instance through the filter is one of the ways to do without global. But in that case can only access it via that filter so only for anti-global purists mostly

Yes, using a filter instead of an action you can make it accessible, I tried that approach sometimes but doesn't seem a very good one, for Brain I've used the classical Container::instance() method, even if an action to allow adding variables to Pimple container (in a very similar way I suggested to you) is also available there.

So the difference is basically the need to check if plugin exists.

And globalize the container var :)