View->context issue vs Yii::$app->modules #1452

Closed
jakobrosenberg opened this Issue Dec 6, 2013 · 22 comments

Comments

Projects
None yet
7 participants
@jakobrosenberg

How do we access module properties from view? Say my module has a navbar and I want the navbar displayed in submodules. So far so good.

Here is the problem, if I want the navbar to display a property from the module, I'm left with two options. I can use echo Yii::$app->modules['MyModule']->myProperty. This works fine, but it forces anyone who uses my extension to use the module name 'MyModule'. This has the potential to cause extension conflicts.

Another option is $this->context->module->myProperty. But here is the next problem. If a submodule is loaded, the context->module refers to the submodule and myProperty is no longer accessible.

At the moment I'm considering attaching my module as a component/module in the module::init() so I can access it through Yii::$app->_myModule. Would that be a correct practice?

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Dec 7, 2013

Member

What kind of submodules are added to your module? If you provide them you can add a method that makes the navbar available in all submodules. If others provide submodules for your module they may extend from a base module class created by the main module.

Member

cebe commented Dec 7, 2013

What kind of submodules are added to your module? If you provide them you can add a method that makes the navbar available in all submodules. If others provide submodules for your module they may extend from a base module class created by the main module.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 7, 2013

Member

What is context here? Is it the navbar widget or controller?

Member

qiangxue commented Dec 7, 2013

What is context here? Is it the navbar widget or controller?

@mgrechanik

This comment has been minimized.

Show comment
Hide comment
@mgrechanik

mgrechanik Dec 7, 2013

Contributor

@jakobrosenberg , if the property name is unique among the module and it's submodules you can try this option:

$property = 'someProperty';
$value = null;
$module = $this->context->module;
do {
    if (isset($module->$property)) {
        $value = $module->$property;
        break;
    }
    $module = $module->module;
}
while($module !== null);
var_dump($value);
Contributor

mgrechanik commented Dec 7, 2013

@jakobrosenberg , if the property name is unique among the module and it's submodules you can try this option:

$property = 'someProperty';
$value = null;
$module = $this->context->module;
do {
    if (isset($module->$property)) {
        $value = $module->$property;
        break;
    }
    $module = $module->module;
}
while($module !== null);
var_dump($value);
@jakobrosenberg

This comment has been minimized.

Show comment
Hide comment
@jakobrosenberg

jakobrosenberg Dec 7, 2013

Context here is the controller and the view file is mymodule/layouts/main.php

@cebe, that would require all submodules to be written explicitely for my module, which so far is the case for none of them.

The same problem occurs in url generation. If the navigation mymodule/layouts/main.php uses ['mycontroller/myaction'] and I include a submodule not specifically tailored to my module. I can fix it by including my module in the url ['/mymodule/mycontroller/myaction] but then it prevents anyone from using it as a submodule or under a different name.

In your basic app layout you handle this with ['label' => 'Home', 'url' => ['/site/index']] because obviously, the main app should be able to locate it's own routes regardless of the which modules we're currently visiting. Some modules have the same requirements, they provide their own layout which needs to work regardless of parent/nested modules.

@mgrechanik, I thought about extending the view and creating a function like that, similar to jQuery's closest("#specific_parent"). It feels like a hack though and I don't know if it would end up too convoluted if it has to work with urls. I'll give it a shot and see how it works.

Context here is the controller and the view file is mymodule/layouts/main.php

@cebe, that would require all submodules to be written explicitely for my module, which so far is the case for none of them.

The same problem occurs in url generation. If the navigation mymodule/layouts/main.php uses ['mycontroller/myaction'] and I include a submodule not specifically tailored to my module. I can fix it by including my module in the url ['/mymodule/mycontroller/myaction] but then it prevents anyone from using it as a submodule or under a different name.

In your basic app layout you handle this with ['label' => 'Home', 'url' => ['/site/index']] because obviously, the main app should be able to locate it's own routes regardless of the which modules we're currently visiting. Some modules have the same requirements, they provide their own layout which needs to work regardless of parent/nested modules.

@mgrechanik, I thought about extending the view and creating a function like that, similar to jQuery's closest("#specific_parent"). It feels like a hack though and I don't know if it would end up too convoluted if it has to work with urls. I'll give it a shot and see how it works.

@mgrechanik

This comment has been minimized.

Show comment
Hide comment
@mgrechanik

mgrechanik Dec 7, 2013

Contributor

Maybe checking for a module's class name by get_class would be in a little better way than checking for some unique property name. (this concerns this "closest" solution).

From your explanation I have not understood what you actually expect from urls.

Contributor

mgrechanik commented Dec 7, 2013

Maybe checking for a module's class name by get_class would be in a little better way than checking for some unique property name. (this concerns this "closest" solution).

From your explanation I have not understood what you actually expect from urls.

@jakobrosenberg

This comment has been minimized.

Show comment
Hide comment
@jakobrosenberg

jakobrosenberg Dec 7, 2013

I think ID would be better in this case as module::id is already set from the class name and it allows overriding the ID if that should ever be required.

@coredevs could you possibly add one of these two options to yii\web\Controller?

Option 1

    /**
     * Traverses through the module and its ancestors and returns the first module with a matching $id. Returns the app if not found.
     * @param $id
     * @return Module
     */
    public function module($id)
    {
        $module = $this->module;
        while ($module->module && $module->id !== $id){
            $module = $module->module;
        }
        return $module;
    }

Option 2

    /**
     * Traverses through the module and its ancestors and returns the first module with a matching $id. Returns null if not found.
     * @param $id
     * @return null|Module
     */
    public function module($id)
    {
        $module = $this->module;
        while ($module->id !== $id){
            if(($module->module)){
                $module = $module->module;
            }else {
                return null;
            }
        }
        return $module;
    }

It doesn't break functionality of the current module property, but allows us to use the existing $this->context->module to find the parent module and $this->context->module($id) to traverse to a specific module. This would be a very useful feature in modules with layouts.

I think ID would be better in this case as module::id is already set from the class name and it allows overriding the ID if that should ever be required.

@coredevs could you possibly add one of these two options to yii\web\Controller?

Option 1

    /**
     * Traverses through the module and its ancestors and returns the first module with a matching $id. Returns the app if not found.
     * @param $id
     * @return Module
     */
    public function module($id)
    {
        $module = $this->module;
        while ($module->module && $module->id !== $id){
            $module = $module->module;
        }
        return $module;
    }

Option 2

    /**
     * Traverses through the module and its ancestors and returns the first module with a matching $id. Returns null if not found.
     * @param $id
     * @return null|Module
     */
    public function module($id)
    {
        $module = $this->module;
        while ($module->id !== $id){
            if(($module->module)){
                $module = $module->module;
            }else {
                return null;
            }
        }
        return $module;
    }

It doesn't break functionality of the current module property, but allows us to use the existing $this->context->module to find the parent module and $this->context->module($id) to traverse to a specific module. This would be a very useful feature in modules with layouts.

@jakobrosenberg

This comment has been minimized.

Show comment
Hide comment
@jakobrosenberg

jakobrosenberg Dec 7, 2013

I concur. Get_class() is a better option as $id wasn't as flexible as I thought.

    /**
     * Traverses through the module and its ancestors and returns the first module that matches $class. Returns null if not found.
     * @param string $class
     * @return null|\yii\base\Module
     */
    public function module($class)
    {
        $module = $this->module;
        while (get_class($module) !== $class){
            if(($module->module)){
                $module = $module->module;
            }else {
                return null;
            }
        }
        return $module;
    }

I concur. Get_class() is a better option as $id wasn't as flexible as I thought.

    /**
     * Traverses through the module and its ancestors and returns the first module that matches $class. Returns null if not found.
     * @param string $class
     * @return null|\yii\base\Module
     */
    public function module($class)
    {
        $module = $this->module;
        while (get_class($module) !== $class){
            if(($module->module)){
                $module = $module->module;
            }else {
                return null;
            }
        }
        return $module;
    }
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 8, 2013

Member

@jakobrosenberg Could you please answer my question? "What is context here? Is it the navbar widget or controller?"

Member

qiangxue commented Dec 8, 2013

@jakobrosenberg Could you please answer my question? "What is context here? Is it the navbar widget or controller?"

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Dec 9, 2013

Member

@qiangxue it is in this comment: #1452 (comment)

Context here is the controller and the view file is mymodule/layouts/main.php

Member

cebe commented Dec 9, 2013

@qiangxue it is in this comment: #1452 (comment)

Context here is the controller and the view file is mymodule/layouts/main.php

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 9, 2013

Member

Thanks, @cebe. I didn't notice this line.

Because a layout view is shared by different controllers/modules/actions, if something external needs to be accessed in the layout, it has to be global. A natural choice is the application instance. You can define the module property to set an application parameter, and within the layout, you can access the application parameter instead of trying to find out the module.

I don't think we should add the module search function in the core, unless there are other convincing use cases for it.

Member

qiangxue commented Dec 9, 2013

Thanks, @cebe. I didn't notice this line.

Because a layout view is shared by different controllers/modules/actions, if something external needs to be accessed in the layout, it has to be global. A natural choice is the application instance. You can define the module property to set an application parameter, and within the layout, you can access the application parameter instead of trying to find out the module.

I don't think we should add the module search function in the core, unless there are other convincing use cases for it.

@qiangxue qiangxue closed this Dec 9, 2013

@jakobrosenberg

This comment has been minimized.

Show comment
Hide comment
@jakobrosenberg

jakobrosenberg Dec 9, 2013

@qiangxue that's true, but it doesn't solve the url handling.

Here's my use case;

$module = $this->context->module('MyModule');
$path = "/$module->uniqueId/";

Nav::Widget([
    'items' => [
        ['label'=>'Index', 'url'=>[$path.'default/index']],
        ['label'=>'Index', 'url'=>[$path.'submodule/default/index']]

Without the path, any child modules nested in the layout would mess up the urls. If you're creating an extension, hardcoding the urls aren't an option, unless you want to force users of your extension to use a predefined route for the module.

Either modules shouldn't affect layout files of parent modules or there should be a way to get the route for the parent module. If not, anyone who wants to use a layout for a child module will have to come up with a hack. It's a lot of reinventing the wheel and browsing through source code.

Layouts are not relative to the controller like other view files, they're relative to their specific module and should have a way of accessing it without using a hack. This applies in most, if not all cases. The app main layout has hardcoded urls for this very reason, that layout urls are not relative to nested modules.

@qiangxue that's true, but it doesn't solve the url handling.

Here's my use case;

$module = $this->context->module('MyModule');
$path = "/$module->uniqueId/";

Nav::Widget([
    'items' => [
        ['label'=>'Index', 'url'=>[$path.'default/index']],
        ['label'=>'Index', 'url'=>[$path.'submodule/default/index']]

Without the path, any child modules nested in the layout would mess up the urls. If you're creating an extension, hardcoding the urls aren't an option, unless you want to force users of your extension to use a predefined route for the module.

Either modules shouldn't affect layout files of parent modules or there should be a way to get the route for the parent module. If not, anyone who wants to use a layout for a child module will have to come up with a hack. It's a lot of reinventing the wheel and browsing through source code.

Layouts are not relative to the controller like other view files, they're relative to their specific module and should have a way of accessing it without using a hack. This applies in most, if not all cases. The app main layout has hardcoded urls for this very reason, that layout urls are not relative to nested modules.

@qiangxue qiangxue reopened this Dec 9, 2013

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 9, 2013

Member

Will consider exposing the owner module to the layout view.

Member

qiangxue commented Dec 9, 2013

Will consider exposing the owner module to the layout view.

@qiangxue qiangxue added this to the 2.0 RC milestone Apr 16, 2014

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Jun 26, 2014

Member

@yiisoft/core-developers I'm thinking about adding a static property yii\base\Module::$instance.

The issue reported here is a more generic one, not limited to layouts. Code within a module may need to access the module instance. Currently, this can only be done through Controller::module. However, when there are nested modules, this approach is not safe because you may get a child module instance.

The drawback of having Module::$instance is during a request, there could be multiple instances of the same module. But I think this is extremely rare and can be ignored.

What do you think?

Member

qiangxue commented Jun 26, 2014

@yiisoft/core-developers I'm thinking about adding a static property yii\base\Module::$instance.

The issue reported here is a more generic one, not limited to layouts. Code within a module may need to access the module instance. Currently, this can only be done through Controller::module. However, when there are nested modules, this approach is not safe because you may get a child module instance.

The drawback of having Module::$instance is during a request, there could be multiple instances of the same module. But I think this is extremely rare and can be ignored.

What do you think?

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jun 26, 2014

Member

at which point will this instance be populated? is it something like Yii::$app which is set in constructor of application class?

Member

cebe commented Jun 26, 2014

at which point will this instance be populated? is it something like Yii::$app which is set in constructor of application class?

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Jun 26, 2014

Member

It will be populated when Module::getModule() is called the first time for a module.

Member

qiangxue commented Jun 26, 2014

It will be populated when Module::getModule() is called the first time for a module.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jun 26, 2014

Member

sounds okay to me.

Member

cebe commented Jun 26, 2014

sounds okay to me.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jun 26, 2014

Member

Is it always a module whose controller is currently running or not? getModule() could be used on a child module. Wouldn't that create a mess?

Member

samdark commented Jun 26, 2014

Is it always a module whose controller is currently running or not? getModule() could be used on a child module. Wouldn't that create a mess?

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jun 27, 2014

Member

Well, of course we have to set CustomModelClass::$instance for each module than the static property of the base Module.

Member

cebe commented Jun 27, 2014

Well, of course we have to set CustomModelClass::$instance for each module than the static property of the base Module.

@qiangxue qiangxue closed this in 8e96623 Jun 27, 2014

@zelenin

This comment has been minimized.

Show comment
Hide comment
@zelenin

zelenin Jul 2, 2014

Contributor

How I can get module ID in bootstrap file? Module not initialised yet and no way get.ID

    public function bootstrap($app)
    {
        if ($app instanceof \yii\web\Application && $module = Module::getInstance()) {
            $moduleId = $module->id; // not work because $module === null
            $app->getUrlManager()->addRules([
                'translations/<id:\d+>' => $moduleId . '/default/update',
                'translations' => $moduleId . '/default/index',
            ], false);
        }
    }
Contributor

zelenin commented Jul 2, 2014

How I can get module ID in bootstrap file? Module not initialised yet and no way get.ID

    public function bootstrap($app)
    {
        if ($app instanceof \yii\web\Application && $module = Module::getInstance()) {
            $moduleId = $module->id; // not work because $module === null
            $app->getUrlManager()->addRules([
                'translations/<id:\d+>' => $moduleId . '/default/update',
                'translations' => $moduleId . '/default/index',
            ], false);
        }
    }
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jul 2, 2014

Member

this feature is to access the module when it is currently active i.e. running a controller action. Getting the id in bootstrap is not possible because it is not initialized.

Member

cebe commented Jul 2, 2014

this feature is to access the module when it is currently active i.e. running a controller action. Getting the id in bootstrap is not possible because it is not initialized.

@dizews

This comment has been minimized.

Show comment
Hide comment
@dizews

dizews Jul 2, 2014

Contributor

@zelenin you can use:

$app->getUrlManager()->addRules([
      $this->id => $this->id,
      $this->id . '/<controller:\w+>/<action:\w+>' => $this->id . '/<controller>/<action>',
], false);
Contributor

dizews commented Jul 2, 2014

@zelenin you can use:

$app->getUrlManager()->addRules([
      $this->id => $this->id,
      $this->id . '/<controller:\w+>/<action:\w+>' => $this->id . '/<controller>/<action>',
], false);
@zelenin

This comment has been minimized.

Show comment
Hide comment
Contributor

zelenin commented Jul 2, 2014

tvdavid pushed a commit to tvdavid/yii2 that referenced this issue Jul 24, 2014

Fixes #1452: Added `Module::getInstance()` to allow accessing the mo…
…dule instance from anywhere within the module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment