silverstripe-admin icon indicating copy to clipboard operation
silverstripe-admin copied to clipboard

Extension orders when extending LeftAndMain::getClientConfig

Open robbieaverill opened this issue 5 years ago • 0 comments

SS 4.2.x-dev

Note that this method is marked as an experimental API, so we probably have more flexibility to change it.

Given I have a subclass of LeftAndMain that I want to add extra client config for React data schemas to, I have two options to extend this:

  1. overloading getClientConfig in my subclass
  2. adding an Extension which implements updateClientConfig

Option 1: Overloading

So I update my code to look like this:

public function getClientConfig()
{
    $config = parent::getClientConfig(); // note: extension point has already been executed now
    $config['form']['myForm'] = [
        'schemaUrl' => $this->Link('schema/myForm'),
    ];
    return $config;
}

The problem with this approach is that the extension point updateClientConfig has already been run in parent::getClientConfig(); which makes further extensibility not possible on myForm.

Option 2: Extension to update config

class MyExtension extends Extension
{
    public function updateClientConfig(&$config)
    {
        $config['form']['myForm'] = [
            'schemaUrl' => $this->Link('schema/myForm'),
        ];
    }
}

This option is better, because it's isolated in its own extension class and can be controlled in user code individually, but the issue is that (A) it's not part of the controller any more, where it probably should be, and that the approach to doing this isn't documented as the standard practice. If it is, then this could just become a docs task.


Perhaps we need a different method that devs should implement to define their schema, for example:

public function getClientConfig()
{
    $config = $this->provideClientConfig();
    $this->extend('updateClientConfig', $config);
    return $config;
}

/**
 * Overload this method to add your own controller's custom schema definitions, ensure you call parent::provideClientConfig();
 * 
 * @return array
 */
protected function provideClientConfig()
{
    return [
        // defaults from LeftAndMain::getClientConfig
    ];
}

This would ensure that both option 1 and 2 can be used depending on how devs want to use this experimental API. It could also be implemented in a backwards compatible way.

robbieaverill avatar Jul 22 '18 22:07 robbieaverill