themosis / framework

The Themosis framework core.
https://framework.themosis.com/
GNU General Public License v2.0
671 stars 121 forks source link

Unable to override or register class aliases #254

Closed grahamblevins closed 8 years ago

grahamblevins commented 8 years ago

In 1.2.2, the themosisClassAliases filter appears to be applied in the themosis plugin before the filter is added in the themosis theme.

What is the best way to override or register class aliases?

jlambe commented 8 years ago

Hmm indeed there is bug with this. It seems to be an issue of code order. I've made a quick patch so perhaps you can test it within your project and let me know how it goes so I'll be able to bring the patch for minor release 1.2.3.

1-Inside the themosis-framework/bootstrap/start.php file, wrap the class aliases code within a callback attached to the after_setup_theme hook like so:

add_action('after_setup_theme', function()
{
    $aliases = apply_filters('themosisClassAliases', [
        'Themosis\\Facades\\Action'                 => 'Action',
        'Themosis\\Facades\\Ajax'                   => 'Ajax',
        'Themosis\\Facades\\Asset'                  => 'Asset',
        'Themosis\\Facades\\Config'                 => 'Config',
        'Themosis\\Route\\Controller'               => 'Controller',
        'Themosis\\Facades\\Field'                  => 'Field',
        'Themosis\\Facades\\Form'                   => 'Form',
        'Themosis\\Facades\\Html'                   => 'Html',
        'Themosis\\Facades\\Input'                  => 'Input',
        'Themosis\\Metabox\\Meta'                   => 'Meta',
        'Themosis\\Facades\\Metabox'                => 'Metabox',
        'Themosis\\Page\\Option'                    => 'Option',
        'Themosis\\Facades\\Page'                   => 'Page',
        'Themosis\\Facades\\PostType'               => 'PostType',
        'Themosis\\Facades\\Route'                  => 'Route',
        'Themosis\\Facades\\Section'                => 'Section',
        'Themosis\\Session\\Session'                => 'Session',
        'Themosis\\Taxonomy\\TaxField'              => 'TaxField',
        'Themosis\\Taxonomy\\TaxMeta'               => 'TaxMeta',
        'Themosis\\Facades\\Taxonomy'               => 'Taxonomy',
        'Themosis\\Facades\\User'                   => 'User',
        'Themosis\\Facades\\Validator'              => 'Validator',
        'Themosis\\Facades\\Loop'                   => 'Loop',
        'Themosis\\Facades\\View'                   => 'View'
    ]);

    foreach ($aliases as $namespace => $className)
    {
        class_alias($namespace, $className);
    }
});

2-Inside your theme bootstrap/start.php file, locate the code that handles Theme supports, then add the following code:

add_filter('themosisClassAliases', function($aliases)
{
    $as = Themosis\Facades\Config::get('application.aliases');
    $aliases = array_merge($aliases, $as);
    return $aliases;
}, 1);

3-Just under the aliases filter, there are the lines responsible to load all files from the admin directory. Wrap the code within a after_setup_theme hook callback:

add_action('after_setup_theme', function()
{
    $adminPath = themosis_path('admin');
    new Themosis\Core\AdminLoader($adminPath);
});

Make sure to add your aliases inside the theme application.config.php file under the aliases key.

This is for testing now, it should work but let me know how it goes during development. I'll test on my side and I'll provide the fix for next minor release 1.2.3.

grahamblevins commented 8 years ago

The following filter already exists on line 47 of the theme's bootstrap/start.php. Are you saying that it needs to be moved down the file to where the theme support code begins? I don't understand why this is necessary. Thanks for clarifying.

add_filter('themosisClassAliases', function($aliases)
{
    $as = Themosis\Facades\Config::get('application.aliases');
    $aliases = array_merge($aliases, $as);
    return $aliases;
});
jlambe commented 8 years ago

Just update the existing one, my mistake. Le 9 janv. 2016 00:51, "Graham Blevins" notifications@github.com a écrit :

The following filter already exists on line 47 of the theme's bootstrap/start.php. Are you saying that it needs to be moved down the file to where the theme support code begins? I don't understand why this is necessary. Thanks for clarifying.

add_filter('themosisClassAliases', function($aliases) { $as = Themosis\Facades\Config::get('application.aliases'); $aliases = array_merge($aliases, $as); return $aliases; });

— Reply to this email directly or view it on GitHub https://github.com/themosis/framework/issues/254#issuecomment-170159075.

grahamblevins commented 8 years ago

The patch above appears to be working.

In order to override the default aliases, I changed the order of the key / values of the $aliases array, where the class alias is now the key and the namespace is now the value. I don't see any other issues as a result of this change from my initial tests.

add_action('after_setup_theme', function()
{
    $aliases = apply_filters('themosisClassAliases', [
        'Action'                 => 'Themosis\\Facades\\Action',
        'Ajax'                   => 'Themosis\\Facades\\Ajax',
        'Asset'                  => 'Themosis\\Facades\\Asset',
        'Config'                 => 'Themosis\\Facades\\Config',
        'Controller'             => 'Themosis\\Route\\Controller',
        'Field'                  => 'Themosis\\Facades\\Field',
        'Form'                   => 'Themosis\\Facades\\Form',
        'Html'                   => 'Themosis\\Facades\\Html',
        'Input'                  => 'Themosis\\Facades\\Input',
        'Meta'                   => 'Themosis\\Metabox\\Meta',
        'Metabox'                => 'Themosis\\Facades\\Metabox',
        'Option'                 => 'Themosis\\Page\\Option',
        'Page'                   => 'Themosis\\Facades\\Page',
        'PostType'               => 'Themosis\\Facades\\PostType',
        'Route'                  => 'Themosis\\Facades\\Route',
        'Section'                => 'Themosis\\Facades\\Section',
        'Session'                => 'Themosis\\Session\\Session',
        'TaxField'               => 'Themosis\\Taxonomy\\TaxField',
        'TaxMeta'                => 'Themosis\\Taxonomy\\TaxMeta',
        'Taxonomy'               => 'Themosis\\Facades\\Taxonomy',
        'User'                   => 'Themosis\\Facades\\User',
        'Validator'              => 'Themosis\\Facades\\Validator',
        'Loop'                   => 'Themosis\\Facades\\Loop',
        'View'                   => 'Themosis\\Facades\\View'
    ]);

    foreach ($aliases as $className => $namespace)
    {
        class_alias($namespace, $className);
    }
});

Given these changes, is there an argument to bring class alias registration back to the theme? No filter required if aliased in the theme's bootstrap/start.php?

jlambe commented 8 years ago

Actually I need to let the core plugin to handle this because I want custom plugin or theme to define their own aliases if needed.

grahamblevins commented 8 years ago

Makes sense. Thanks for the insight.

grahamblevins commented 8 years ago

Circling back around after looking at the latest commits on dev .. should the class alias be the array key and the namespace as the value? How would you go about overriding the class alias if the namespace is the key?

jlambe commented 8 years ago

I haven't yet looked after this issue but indeed I suppose it will be easier to set the alias as the key.

jlambe commented 8 years ago

After more tests, I'll revert back class alias management to the theme like it was before version 1.2.2. Let plugins define their own aliases will break applications and lead to class conflicts. It's like let composer packages define class aliases all over the place.

A theme is the last place where you can handle your data and put them in place. Define alias only on the theme will ease their work.

As for plugins developed with the framework, I'll enforce the use of long/namespaced classes. If developers want shorter/alias for the classes inside their plugin, they must define a use statement and import the class at the top of each file:

<?php

use \Themosis\Facades\PostType;
use \Themosis\Facades\Metabox;
use \Themosis\Facades\Field;

$post = PostType::make('slug', 'Plural', 'Singular')->set();

A modern IDE like PhpStorm for example auto suggests to import classes. This will prevent alias conflict on a project. If a plugin author is defining some "facades" that can be used inside other plugins or theme, they should mention the facade long name in their docs so the user can define or not an alias for its project.