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

Drop AbstractView::initDefaultApp and App::init methods #1968

Merged
merged 10 commits into from
Jan 13, 2023

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Jan 11, 2023

Creating many implicit apps can imply negative performance and/or unexpected (other than configured, like implicit UI persistence comma) behaviour, both hard to debug.

If you call View::invokeInit() manually (not recommended, but needed for edge cases), then you need to set the app manually before the call.

This PR also asserts app is set no later than in init, this requirement may be drop in the future, but currently it is needed to load a template which is defined in almost every View.

Also drop ExecutorFactory::create() method in favor of ExecutorFactory::createExecutor().

@mvorisek mvorisek marked this pull request as ready for review January 13, 2023 20:48
@mvorisek mvorisek changed the title Drop AbstractView::initDefaultApp method Drop AbstractView::initDefaultApp and App::init methods Jan 13, 2023
@@ -68,6 +68,7 @@ public function addActionMenuItem($item, $action = null, string $confirmMsg = ''
$item = Factory::factory([View::class], ['name' => false, 'ui' => 'item', 'content' => $item]);
}

$item->setApp($this->getApp());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory, app can be already set if an object with an app preset is passed

@mvorisek mvorisek merged commit 3ea4884 into develop Jan 13, 2023
@mvorisek mvorisek deleted the no_implicit_app branch January 13, 2023 21:24
@mkrecek234
Copy link
Contributor

@mvorisek Using invokeInit heavily, how can I set app manually? My central app class is set using $app = new App and then $app->invokeInit() is called, as around 50 models require App Scope right away. What is the recommended solution for this BC-break?

@mvorisek
Copy link
Member Author

there should be no or minimal BC break if you use App in Models

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants