Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing modules externally without knowing their ID #14421

Closed
vercotux opened this issue Jul 10, 2017 · 47 comments
Closed

Accessing modules externally without knowing their ID #14421

vercotux opened this issue Jul 10, 2017 · 47 comments

Comments

@vercotux
Copy link

vercotux commented Jul 10, 2017

Consider the following scenario:

  1. Take any module with a '_form' view containing an ActiveForm.
  2. Render module's '_form' outside of module (eg. in main app site/index view). For example: $this->render('@somevendor/somemodule/frontend/views/post/_form', ['model'=>$model]);

The rendered ActiveForm 'action' URL in our module's '_form' will be incorrect no matter what we do:

  • If the form uses a relative route (eg. ['post/create']) then the URL will be pointing to the wrong controller (app's instead of module's).
  • If the form uses a fixed absolute route (eg. ['//module/post/create']) then the URL will be pointing to the potentially wrong module ID (it could be called something other than "module").
  • If we try to dynamically grab the module's ID (eg. by doing ['//'.Module::getInstance()->id.'/post/create']) we will end up with a "Trying to get property of non-object" exception.

The mechanism to access the module externally without knowing its ID in advance seems to be missing and/or is not documented.

@yii-bot yii-bot closed this as completed Jul 10, 2017
@samdark samdark reopened this Jul 10, 2017
@samdark samdark removed the question label Jul 10, 2017
@samdark
Copy link
Member

samdark commented Jul 10, 2017

Sorry. Hit the wrong label...

@yiisoft yiisoft deleted a comment from yii-bot Jul 10, 2017
@samdark
Copy link
Member

samdark commented Jul 10, 2017

I think it would be something like:

$this->controller->module->id . '/controller/action'

@samdark samdark added this to the 2.0.15 milestone Jul 10, 2017
@vercotux
Copy link
Author

vercotux commented Jul 10, 2017

Sadly, that only works inside the modules. Modules also come with widgets and other components which may need to be used externally. If we run that code outside the module $this->controller->module will be null.

@samdark
Copy link
Member

samdark commented Jul 10, 2017

Then these aren't part of modules.

@vercotux
Copy link
Author

vercotux commented Jul 10, 2017

Well they still rely on the modules (eg. they need their controllers like in my original example or they need to read the module's config parameters). But they cannot get to the modules they were shipped along with without knowing in advance what the ID will be, because it is determined by the module user. So unless the module developer has a crystal ball, there's no way to provide the necessary components with that package. Other than hardcoding the ID and then demanding the module user to use that ID, which is just sad.

@samdark
Copy link
Member

samdark commented Jul 10, 2017

In your original example you're rendering a view by providing a path to it. You're not giving any context to what this view belongs to or anything so you have to supply module's ID explicitly. For example, like this:

$this->render('@somevendor/somemodule/frontend/views/post/_form', [
    'model'=>$model,
    'action' => ['/moduleID/controllerID/actionID'],
]);

@samdark
Copy link
Member

samdark commented Jul 10, 2017

Not sure how you'd like to make it better...

@rob006
Copy link
Contributor

rob006 commented Jul 10, 2017

Render module's '_form' outside of module (eg. in main app site/index view). For example: $this->render('@somevendor/somemodule/frontend/views/post/_form', ['model'=>$model]);

It looks like you're going to create really nasty spaghetti code. You should extract your form to widget and pass routes/module ID as widget configuration.

@vercotux
Copy link
Author

vercotux commented Jul 10, 2017

@samdark That would work, it's just inconvenient plus it will only work with modules which are specifically designed to take the 'action' parameter in their views.

EDIT: Another scenario where that would never work: Inter-dependent modules. A module cannot possibly know what another module's ID will be.

I wish there was some sort of a way for modules to "auto-bootstrap" on demand (not on every pageload), whenever one of their dependant components are initialized, then they would always be available via Module::getInstance().

@rob006 Absolutely. I was merely using that to illustrate the issue.

@samdark
Copy link
Member

samdark commented Jul 10, 2017

"auto-bootstrap" on demand isn't possible. Either it's auto or it's on demand. Either you're doing it on each request or triggering it from the code explicitly or implicitly.

@rob006
Copy link
Contributor

rob006 commented Jul 10, 2017

@samdark I think that main problem is that Module::getInstance() does not support instantiating module which makes this method useless in many cases. You can't safely use MyModule::getInstance() in model, because this model can be used outside of module and then getInstance() will return null, so you can't rely on it.

@samdark
Copy link
Member

samdark commented Jul 10, 2017

How would you suggest to solve it?

@rob006
Copy link
Contributor

rob006 commented Jul 10, 2017

IMO getInstance() - should always return module instance or throw exception if module can not be instantiated. We could traverse modules config tree to find config for given class and/or provide some fallback in configuration, like:

return [
	'modulesMap' => [
		MyModule::class => function () {
			return Yii::$app->getModule('my-module');
		},
	],
	// ...
];

@vercotux vercotux changed the title Rendering module forms outside of module Accessing modules externally without knowing their ID Jul 10, 2017
@samdark
Copy link
Member

samdark commented Jul 10, 2017

What if the module is used twice w/ different configs/IDs?

@rob006
Copy link
Contributor

rob006 commented Jul 10, 2017

If module is instantiated - return instance.
If there is a configured callback for module class in modulesMap - return result of callback (this allows to define preferred module).
If not - traverse modules config and pick first configured module with specified class.
When everything fails - throw exception.

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

What @rob006 is proposing is definitely an improvement.
But I agree with @samdark. The module instance returned should never be ambiguous. I'm not a big fan of the "pick first" approach. Perhaps instead there could be another way to explicitly specify the "default" module? Adding something like a static $defaultModule property (or even method) containing the default module ID into yii\base\Module comes to mind, although I'm not sure how a static property like that would be configurable other than overriding. Module devs would still need to hardcode an ID, but at least users would have a way to change it, plus most modules could be made to work without bootstraping which is pretty huge.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

What is wrong with configuring default module via modulesMap?

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

Nothing. I was describing the case where something like moduleMap is not configured by the user (instead of doing a fallback to first in config array). It still has to work out-of-the-box so to speak, without requiring a map.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

If not user, the who should configure default module ID for app?

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

It should be the user. But they already do (as the module config key). Requiring the user to set the exact same ID twice in two different places is just wrong (not to mention it would break every existing app configuration that uses modules out there). It should be mandatory no more than 1 time. In my proposal the user has to do it once, and the second time is optional, only if they want to customize it.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

In my proposal the user has to do it once, and the second time is optional, only if they want to customize it.

Same here - modulesMap is required only if you use the same module more than once (and you don't want to use first as default) or module is not explicitly configured.

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

@bizley Your solution I see was to just bootstrap the entire module. I know I'll get flack for this, but I don't think that is a proper solution, although better than a hardcoded ID, it's more of a workaround. It means your module is being bootstrapped on every pageload, even when it is not required. That sucks.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

Also I don't think Yii should do anything to address the problem of developers keeping hardcoded module's id. All the common solutions of dealing with the module's id are provided. If you are the end user of module you know what is the id you've set. If you are the module's creator you can deal with id from inside the module.

But as a end user I can not use components from these modules outside of them. For example I can not safely use bizley\podium\models\User model because it blindly assumes that Podium module is already instantiated.

@bizley
Copy link
Member

bizley commented Jul 11, 2017

@vercotux Bootstraping there is not a solution to the "finding id" problem (although I admit it's not optimized).
@rob006 You are right but it was never ment to be used outside of the module - you guys are looking for a way to use something that was never designed for.

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

@bizley It's a way to avoid needing an ID entirely at the expense of making your users read the documentation every time they want to include your module just to see whether it needs to be bootstrapped. The design is flawed and we are trying to improve it.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

You are right but it was never ment to be used outside of the module

Why not? If I would like to use topics and posts from your module, as a base for comments engine in my news module, this issue is the main problem I will have.

@bizley
Copy link
Member

bizley commented Jul 11, 2017

@vercotux You are not reading documentation before using 3rd party packages? Anyway, as I said, bootstrapping there is used for internal logs and routing - I'm using id from inside the module so it's not a problem for me, user can set any id. Remove Podium from bootstrap and it will still work (more or less ;)

@rob006 You are right again and now I'm working on separate API for this so you can use separate parts of it (like you want) - but this is the thing: I'm preparing the module to be used like that.

@rob006
Copy link
Contributor

rob006 commented Jul 11, 2017

Is there any other problem to solve besides getInstance()?

@vercotux
Copy link
Author

vercotux commented Jul 11, 2017

@bizley Of course I read documentation. You're missing the point. I read the docs, configure the module and override it so that I can keep re-using it in future projects without studying the documentation all over again. Except I have to go back to the documentation every time anyway just to find and see if I need to also add an ID to that annoying 'bootstrap' array.
Anyway as @rob006 said, most if not all of these problems could be resolved inside getInstance().

@kartik-v
Copy link
Contributor

kartik-v commented Sep 29, 2017

I have addressed it to an extent for my extensions where one needs to use a widget available inside a module outside of the module scope. All the widgets will allow a moduleId to be configured as below so that the module context and settings can be derived if using outside of module scope. This will also address the scenario where different module identifiers are configured for the same module in your Yii configuration.

https://github.com/kartik-v/yii2-krajee-base/blob/0cafe6376780cedcd00f52c9d82b172d5851f6b0/Widget.php#L27-L33

If moduleId is not set it will use the default parsed module instance. For example this is how the Dynagrid widget extension interprets the module:

https://github.com/kartik-v/yii2-dynagrid/blob/3e9ca029df7f7ceae3c7dd7cd923e354eae72263/DynaGrid.php#L436-L450

@schmunk42
Copy link
Contributor

What if the module is used twice w/ different configs/IDs?

Then throw an AmbiguousModuleException :) with an additional configurable property moduleId like above, which has to be configured in that case.
If a widget is used within a module, it should be able to find its correct module.

@alotacents
Copy link

Sorry for commenting on a issue months later, but I just want to share another alternative then using the bootstrap property. You can initialize the know modules needed for a controller by adding a beforeAction event or just get the module in the controller Init function.

in controller class

public function init(){
    parent::init();
if(Yii::$app->hasModule('cfgModuleName')){
            Yii::$app->getModule('cfgModuleName');
        }
or
    $this->on('beforeAction',function($event){
if($event->action->id == 'create'){
        if(Yii::$app->hasModule('cfgModuleName')){
            Yii::$app->getModule('cfgModuleName');
        }
}
    });
}

Now when you run Module::getInstance() it will return the instance of the module. So if you don't want to always load the module on every request via bootstrap. With this simple code addition you now can load by controller or even action if you check the event action id property.

It would be cool if we could bootstrap by route $bootstrap['site/*'=>['cfgModule1','cfgModule2']]; prefer that then to making a moduleMap. The con for bootstrap by route can have a huge performance hit if there is a lot of options. I do have another issue which I can't solve and I would like to be able to get the route in the beforeRequest event. I understand a global bootstrap but if I only want a module to bootstrap for a certain request route there is no option unless in the beforeRequest function you do the whole getRequest and resolve it which is what the next stage in the life cycle is going to do.

@neoacevedo
Copy link

Old thread but making some example, I build a module with a separated bootstrap class and I want to get my module's ID from that class, how do I do if MyModule::getInstance() always returns null?

@alotacents
Copy link

neoacevedo if your module implements BootstrapInterface then you haft to have that component in the application config bootstrap array, which will load the class on application startup. If that is not the case and you just want to have the Class::getInstance() to return you can do my recommendation in the controller extend the init function to load the module class so now when you call Class::getInstance() in actions for the controller it will return a instance since the class has been loaded.

loading via bootsrap

return [
'bootstrap' => ['myModule'],
    'modules' => [
        'myModule' => [
            'class' => 'app\modules\MyModule',
        ],
    ]
]

loading via controller

public function init(){
    parent::init();
   // loading by module name
    if(Yii::$app->hasModule('myModule')){
      Yii::$app->getModule('myModule');
   }
}

@neoacevedo
Copy link

@alotacents by implementing modules with separated Bootstrap class I don't need to configure the bootstrap array with my module.

For your second suggestion, the module controller is called once the bootstrap stage is finished and the module is loaded (or at least is how I have tracked it in XDebug) so, to get the module ID and settings from custom classes as for instance custom UrlRule class, my mentioned Bootstrap class.

I have achieved how to get the module using a trait and it is faster than hardcoding the module ID for what I need - I don't know why is faster -.

@alotacents
Copy link

@neoacevedo I really don't comprehend what you're doing without seeing example code. I was providing the knowledge I know about this. Which is if you call the ModuleClass::getInstance() and it returns null it is because it hasn't been loaded. To load the Module you can use bootstrap config to load it or get the module by a declared ID. If you need the module to edit Application config like routes you can do a call to Yii::$app->getModule('myModule') in a \yii\base\Application::EVENT_BEFORE_REQUEST Event or have it loaded every time via the bootstrap config.

I use the second example for when my Module doesn't load any application configuration like routes but one module needs to know the properties of another module. Example Module B controller needs to know the database prefix from Module A. So I would load module A in Module B Controller so when I call ModuleA::getInstance() I will get the module. It would be the same as calling Yii::$app->getModule('ModuleAConfigName') but without having to do that in each action. This allows me to have a central point to change the config name which is in the init function of the controller.

@neoacevedo
Copy link

I mentioned an example about building own routes. At this example, I have to know the real module ID or won't work.

My code is similar to https://github.com/yii2mod/yii2-cms/blob/master/components/PageUrlRule.php#L33 but in the route, I have to add the module ID because I replace the URL, replacing the module, controller and action for an alias.

I can't post more any code examples because at the moment my code is private, but as I said before, I could achieve to get the module ID (just as the user named it in the app config).

@sdlins
Copy link
Contributor

sdlins commented Oct 15, 2018

Everything get worse when the module (child) is configured inside another module (father) inside a master application where each one have a different 'db' config . Aside use hacks (statics or something else), how can the right $db (set in child) be used by the models inside it? Models would need to be coupled to it and it would not to be good for all situations.

Even if I try use Module::getInstance() inside MyModel::getDb() to return the right db, it does not work in this scenario because getInstance() is looking for Yii::$app->loadedModules[$class] but only the father module is there, not the child.

I will like to see how it will be managed in v3.0.

@samdark
Copy link
Member

samdark commented Oct 18, 2018

/move to yiisoft/yii-core

@sdlins
Copy link
Contributor

sdlins commented Oct 19, 2018

Not all messages were moved.

@samdark samdark closed this as completed Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests