Skip to content
This repository has been archived by the owner on Sep 9, 2019. It is now read-only.

Commit

Permalink
Changed Yii::$app to Yii::getApp()
Browse files Browse the repository at this point in the history
  • Loading branch information
hiqsol committed Aug 20, 2018
1 parent 3e0d450 commit cbe2365
Show file tree
Hide file tree
Showing 32 changed files with 81 additions and 81 deletions.
16 changes: 8 additions & 8 deletions build/controllers/ReleaseController.php
Original file line number Diff line number Diff line change
Expand Up @@ -432,19 +432,19 @@ protected function releaseFramework($frameworkPath, $version)
// adjustments

$this->stdout('prepare classmap...', Console::BOLD);
$this->dryRun || Yii::$app->runAction('classmap', [$frameworkPath]);
$this->dryRun || Yii::getApp()->runAction('classmap', [$frameworkPath]);

This comment has been minimized.

Copy link
@ddziaduch

ddziaduch Aug 24, 2018

Interesting. Why not like in Yii1 -> Yii::app() ? :)

This comment has been minimized.

Copy link
@Rafalsky

Rafalsky Aug 28, 2018

Yii1 -> Yii::app()
Yii2 -> Yii::$app
Yii3 -> Yii::getApp()

You will know the version of Yii used in source code :)

This comment has been minimized.

Copy link
@hiqsol

hiqsol Aug 28, 2018

Author Member

First of all. You should not use Yii::getApp() and it will be removed from core too. Eventually.

This comment has been minimized.

Copy link
@rob006

rob006 Sep 5, 2018

Contributor

@hiqsol So why to change $app to getApp() if you're going to remove this in future? It will only make upgrading harder...

This comment has been minimized.

Copy link
@hiqsol

hiqsol Sep 5, 2018

Author Member

There are several considerations.

Purity. The idea is to make code cleaner as much as possible. But there is a lot of resistance and old methods will be kept for conservative developers.
I mean do you use global variables? Why not cease this practice in the Yii?

Flexibility. There is no $app variable anymore in Yii helper. It allows deferring application creation a bit further. The application is not created forcibly.
In theory, one can have no application while still using most of Yii features. In practice app object is used all over the code but it can be changed. It takes time, but it will be done eventually.

In most cases Yii::$app should be changed to $this->app which is much better.

This comment has been minimized.

Copy link
@rob006

rob006 Sep 5, 2018

Contributor

Honestly I have many doubts about this change, especially performance-related. Is there any discussion/explanation how this suppose to work? Some roadmap, benchmark or example app (something more than hello world) maybe? What about static helpers, AR or other objects which does not belong to app directly, but sometimes needs to access app component (like URL manager)? How access app component if you're going to remove static access to app?

This comment has been minimized.

Copy link
@hiqsol

hiqsol Sep 5, 2018

Author Member

Is there any discussion/explanation how this suppose to work?

Please see yii\base\Controller. Compare with yii2. The app is passed with the constructor and then used instead of Yii::$app. This is a general idea. Unfortunately, it cannot be done that easy everywhere that's why I say that there is still a lot of work ahead.
And of course, there is no easy way for static helpers. They will use Yii::getApp() for the moment.
It was discussed to redo static helpers to services but nothing is done in this direction yet.

example app (something more than hello world) maybe?

We plan to redo our applications with Yii 3.0 as soon as possible. E.g. asset-packagist. Hope it will be good enough example.

This comment has been minimized.

Copy link
@rob006

rob006 Sep 5, 2018

Contributor

And of course, there is no easy way for static helpers. They will use Yii::getApp() for the moment.
It was discussed to redo static helpers to services but nothing is done in this direction yet.

And I'm asking exactly about these cases. If you don't even have an idea how to solve this, the whole change is doubtful and it is not going to anywhere. We really need complete plan if we're going to go further with this.

This comment has been minimized.

Copy link
@hiqsol

hiqsol Sep 5, 2018

Author Member

It looks like on one hand you want to keep everything as is while on other "we really need a complete plan" for changes.
And actually, this is what I and most other people want. That's why the changes are small, gradual and completely compatible with yii2 approaches. Those who prefer to get application statically will still be able to do it.

Change Yii::$app -> Yii::getApp() in the whole codebase takes seconds in decent editor. I don't think it's a problem at all. Even more, those who want Yii::$app can have it easily with own Yii class extended from BaseYii. But IMHO it should not be provided (recommended) by Yii.

Yii is already good enough as it is but that doesn't mean that nothing should ever be changed.

and it is not going to anywhere

If it doesn't work in some places it doesn't mean it doesn't work at all.

I think it's better to return to the constructive discussion.
I'm really missing your point. Do you have better ideas?

This comment has been minimized.

Copy link
@rob006

rob006 Sep 5, 2018

Contributor

It looks like on one hand you want to keep everything as is while on other "we really need a complete plan" for changes.

I'm fine with both ways: "Lets keep it as is and use Yii::$app everywhere" and "Get rid of static app completely". The only way I don't like is something in the middle - trying to remove static app instance, but actually not doing it (and don't having a idea how to do this). And it look like we we're going to get third option - app instance is injected in some places using DI, but most of core code uses static instance, since there is no idea how to do it without static instance.

If it doesn't work in some places it doesn't mean it doesn't work at all.

It's like building a plane by focusing on making it drive in the first place. Obviously plane needs to drive in some way before its start flying, but if you focus only on driving, it is easy to end up with a car. And it is pretty hard to turn a car into a plane.

This comment has been minimized.

Copy link
@hiqsol

hiqsol Sep 5, 2018

Author Member

The problem with the big changes that they are ... big :)
There is no option for eating an elephant but one bite at a time.

I like your analogy with car and plane. But it is not exact. A car is a whole system and can not be split.
While the framework can. And one can use only plane parts and fly :)
While car guys can continue to drive on a ground.
Let a thousand flowers bloom. Even after finishing cleaning framework from global variables and service locator I think it is good to allow to use these convenient and familiar conceptions.

Anyway, it's an important discussion and it deserves to be continued in its own issue and not forgotten in depth of commits.
Please create an issue I think other Yii-team members have something to add.

This comment has been minimized.

Copy link
@rob006

rob006 Sep 5, 2018

Contributor

Solutions that are perfectly fine for driving may be completely useless for flying - this is exactly what we are having here with static instance. Maybe current approach is a dead end and we would need to try something completely different to actually fix this issue (get rid of static app instance). It would be better to know that before we've make more BC braking changes which will be completely pointless, since it does not bring us closer to solution.

So yes, please open an issue with description how static app instance will be removed from helpers or entities, and what are the benefits from this change. It should be more productive to stop and think for a moment how this is suppose to work, instead of making more changes without any plan :).

This comment has been minimized.

Copy link
@ddziaduch

ddziaduch Sep 5, 2018

@rob006 @hiqsol I have created an issue for you #23

$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout('updating mimetype magic file and mime aliases...', Console::BOLD);
$this->dryRun || Yii::$app->runAction('mime-type', ["$frameworkPath/helpers/mimeTypes.php"], ["$frameworkPath/helpers/mimeAliases.php"]);
$this->dryRun || Yii::getApp()->runAction('mime-type', ["$frameworkPath/helpers/mimeTypes.php"], ["$frameworkPath/helpers/mimeAliases.php"]);
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout("fixing various PHPDoc style issues...\n", Console::BOLD);
$this->dryRun || Yii::$app->runAction('php-doc/fix', [$frameworkPath]);
$this->dryRun || Yii::getApp()->runAction('php-doc/fix', [$frameworkPath]);
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout("updating PHPDoc @property annotations...\n", Console::BOLD);
$this->dryRun || Yii::$app->runAction('php-doc/property', [$frameworkPath]);
$this->dryRun || Yii::getApp()->runAction('php-doc/property', [$frameworkPath]);
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout('sorting changelogs...', Console::BOLD);
Expand Down Expand Up @@ -565,13 +565,13 @@ protected function releaseApplication($name, $path, $version)

$this->stdout("fixing various PHPDoc style issues...\n", Console::BOLD);
$this->setAppAliases($name, $path);
$this->dryRun || Yii::$app->runAction('php-doc/fix', [$path, 'skipFrameworkRequirements' => true]);
$this->dryRun || Yii::getApp()->runAction('php-doc/fix', [$path, 'skipFrameworkRequirements' => true]);
$this->resetAppAliases();
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout("updating PHPDoc @property annotations...\n", Console::BOLD);
$this->setAppAliases($name, $path);
$this->dryRun || Yii::$app->runAction('php-doc/property', [$path, 'skipFrameworkRequirements' => true]);
$this->dryRun || Yii::getApp()->runAction('php-doc/property', [$path, 'skipFrameworkRequirements' => true]);
$this->resetAppAliases();
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

Expand Down Expand Up @@ -682,11 +682,11 @@ protected function releaseExtension($name, $path, $version)
// adjustments

$this->stdout("fixing various PHPDoc style issues...\n", Console::BOLD);
$this->dryRun || Yii::$app->runAction('php-doc/fix', [$path]);
$this->dryRun || Yii::getApp()->runAction('php-doc/fix', [$path]);
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout("updating PHPDoc @property annotations...\n", Console::BOLD);
$this->dryRun || Yii::$app->runAction('php-doc/property', [$path]);
$this->dryRun || Yii::getApp()->runAction('php-doc/property', [$path]);
$this->stdout("done.\n", Console::FG_GREEN, Console::BOLD);

$this->stdout('sorting changelogs...', Console::BOLD);
Expand Down
4 changes: 2 additions & 2 deletions move-tests/behaviors/AttributeBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ public function setUp()
'name' => 'string',
'alias' => 'string',
];
Yii::$app->getDb()->createCommand()->createTable('test_attribute', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_attribute', $columns)->execute();
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
}

Expand Down
2 changes: 1 addition & 1 deletion move-tests/behaviors/AttributeTypecastBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected function setUp()
'isActive' => 'boolean',
'callback' => 'string',
];
Yii::$app->getDb()->createCommand()->createTable('test_attribute_typecast', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_attribute_typecast', $columns)->execute();
}

protected function tearDown()
Expand Down
4 changes: 2 additions & 2 deletions move-tests/behaviors/AttributesBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ public function setUp()
'name' => 'string',
'alias' => 'string',
];
Yii::$app->getDb()->createCommand()->createTable('test_attribute', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_attribute', $columns)->execute();
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
}

Expand Down
6 changes: 3 additions & 3 deletions move-tests/behaviors/BlameableBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ public function setUp()
'created_by' => 'integer',
'updated_by' => 'integer',
];
Yii::$app->getDb()->createCommand()->createTable('test_blame', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_blame', $columns)->execute();

$this->getUser()->login(10);
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
gc_enable();
gc_collect_cycles();
Expand All @@ -66,7 +66,7 @@ public function tearDown()
*/
private function getUser()
{
return Yii::$app->get('user');
return Yii::getApp()->get('user');
}

public function testInsertUserIsGuest()
Expand Down
2 changes: 1 addition & 1 deletion move-tests/behaviors/CacheableWidgetBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class DynamicCacheableWidget extends BaseCacheableWidget
*/
public function run()
{
$dynamicContentsExpression = 'return "dynamic contents: " . \Yii::$app->params["counter"]++;';
$dynamicContentsExpression = 'return "dynamic contents: " . \Yii::getApp()->params["counter"]++;';
$dynamicContents = $this->view->renderDynamic($dynamicContentsExpression);
$content = '<div>' . $dynamicContents . '</div>';

Expand Down
24 changes: 12 additions & 12 deletions move-tests/behaviors/OptimisticLockBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,18 @@ public function setUp()
'id' => 'pk',
'version' => 'integer NOT NULL',
];
Yii::$app->getDb()->createCommand()->createTable('test_auto_lock_version', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_auto_lock_version', $columns)->execute();

$columns = [
'id' => 'pk',
'version' => 'string NOT NULL',
];
Yii::$app->getDb()->createCommand()->createTable('test_auto_lock_version_string', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_auto_lock_version_string', $columns)->execute();
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
gc_enable();
gc_collect_cycles();
Expand Down Expand Up @@ -100,7 +100,7 @@ public function testNewRecord()
// create a record without any version

$request = new Request();
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

ActiveRecordLockVersion::$behaviors = [
OptimisticLockBehavior::class,
Expand All @@ -113,7 +113,7 @@ public function testNewRecord()
// create a record starting from version 5

$request->setParsedBody(['version' => 5]);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$model = new ActiveRecordLockVersion();
$model->save(false);
Expand All @@ -125,7 +125,7 @@ public function testNewRecord()
public function testUpdateRecord()
{
$request = new Request();
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

ActiveRecordLockVersion::$behaviors = [
OptimisticLockBehavior::class,
Expand Down Expand Up @@ -155,7 +155,7 @@ public function testUpdateRecord()
// save stale data by sending an outdated version

$request->setParsedBody(['version' => 0]);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$thrown = false;

Expand All @@ -171,7 +171,7 @@ public function testUpdateRecord()
// save stale data by sending an 'invalid' version number

$request->setParsedBody(['version' => 'yii']);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$thrown = false;

Expand All @@ -191,7 +191,7 @@ public function testUpdateRecord()
// a successful update by sending the correct version

$request->setParsedBody(['version' => '1']);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$model->save(false);

Expand All @@ -201,7 +201,7 @@ public function testUpdateRecord()
public function testDeleteRecord()
{
$request = new Request();
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

ActiveRecordLockVersion::$behaviors = [
OptimisticLockBehavior::class,
Expand Down Expand Up @@ -229,7 +229,7 @@ public function testDeleteRecord()
// delete stale data by sending an outdated version

$request->setParsedBody(['version' => 0]);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$thrown = false;

Expand All @@ -245,7 +245,7 @@ public function testDeleteRecord()
// a successful update by sending the correct version

$request->setParsedBody(['version' => '1']);
Yii::$app->set('request', $request);
Yii::getApp()->set('request', $request);

$model->delete();

Expand Down
6 changes: 3 additions & 3 deletions move-tests/behaviors/SluggableBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,18 @@ public function setUp()
'category_id' => 'integer',
'belongs_to_id' => 'integer',
];
Yii::$app->getDb()->createCommand()->createTable('test_slug', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_slug', $columns)->execute();

$columns = [
'id' => 'pk',
'name' => 'string',
];
Yii::$app->getDb()->createCommand()->createTable('test_slug_related', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_slug_related', $columns)->execute();
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
gc_enable();
gc_collect_cycles();
Expand Down
6 changes: 3 additions & 3 deletions move-tests/behaviors/TimestampBehaviorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ public function setUp()
'created_at' => 'integer NOT NULL',
'updated_at' => 'integer',
];
Yii::$app->getDb()->createCommand()->createTable('test_auto_timestamp', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_auto_timestamp', $columns)->execute();

$columns = [
'id' => 'pk',
'created_at' => 'string NOT NULL',
'updated_at' => 'string',
];
Yii::$app->getDb()->createCommand()->createTable('test_auto_timestamp_string', $columns)->execute();
Yii::getApp()->getDb()->createCommand()->createTable('test_auto_timestamp_string', $columns)->execute();
}

public function tearDown()
{
Yii::$app->getDb()->close();
Yii::getApp()->getDb()->close();
parent::tearDown();
gc_enable();
gc_collect_cycles();
Expand Down
8 changes: 4 additions & 4 deletions move-tests/db/DataColumnTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ public function testFilterInput_Array()
'id' => 'pk',
'customer_id' => 'integer',
];
ActiveRecord::$db = Yii::$app->getDb();
Yii::$app->getDb()->createCommand()->createTable(Order::tableName(), $columns)->execute();
ActiveRecord::$db = Yii::getApp()->getDb();
Yii::getApp()->getDb()->createCommand()->createTable(Order::tableName(), $columns)->execute();

$filterInput = [1, 2];
$grid = new GridView([
Expand Down Expand Up @@ -166,8 +166,8 @@ public function testFilterInput_FormatBoolean()
'id' => 'pk',
'customer_id' => 'integer',
];
ActiveRecord::$db = Yii::$app->getDb();
Yii::$app->getDb()->createCommand()->createTable(Order::tableName(), $columns)->execute();
ActiveRecord::$db = Yii::getApp()->getDb();
Yii::getApp()->getDb()->createCommand()->createTable(Order::tableName(), $columns)->execute();

$grid = new GridView([
'dataProvider' => new ArrayDataProvider([
Expand Down
2 changes: 1 addition & 1 deletion move-tests/db/test/ActiveFixtureTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public function setUp()
{
parent::setUp();
$db = $this->getConnection();
\Yii::$app->set('db', $db);
\Yii::getApp()->set('db', $db);
ActiveRecord::$db = $db;
}

Expand Down
2 changes: 1 addition & 1 deletion move-tests/db/validators/ExistValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protected function setUp()
{
parent::setUp();

// destroy application, Validator must work without Yii::$app
// destroy application, Validator must work without Yii::getApp()
$this->destroyApplication();
ActiveRecord::$db = $this->getConnection();
}
Expand Down
2 changes: 1 addition & 1 deletion move-tests/db/validators/UniqueValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ protected function setUp()
parent::setUp();
ActiveRecord::$db = $this->getConnection();

// destroy application, Validator must work without Yii::$app
// destroy application, Validator must work without Yii::getApp()
$this->destroyApplication();
}

Expand Down
10 changes: 5 additions & 5 deletions move-tests/view/ThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public function testApplyToEmptyPathMap()
$theme = new Theme(['basePath' => '@app/framework/base/fixtures/themes/basic']);
$expected = Yii::getAlias('@app/framework/base/fixtures/themes/basic/views/site/index.php');

$actual = $theme->applyTo(Yii::$app->basePath . '/views/site/index.php');
$actual = $theme->applyTo(Yii::getApp()->basePath . '/views/site/index.php');

$this->assertPathEquals($expected, $actual);
}
Expand All @@ -112,7 +112,7 @@ public function testApplyToFilledPathMap()
$theme = new Theme($config);
$expected = Yii::getAlias('@app/framework/base/fixtures/themes/basic/views/site/index.php');

$actual = $theme->applyTo(Yii::$app->basePath . '/views/site/index.php');
$actual = $theme->applyTo(Yii::getApp()->basePath . '/views/site/index.php');

$this->assertPathEquals($expected, $actual);
}
Expand All @@ -130,7 +130,7 @@ public function testApplyToFilledPathMapNotExistsViewInFirstTheme()
$theme = new Theme($config);
$expected = Yii::getAlias('@app/framework/base/fixtures/themes/christmas/views/site/main.php');

$actual = $theme->applyTo(Yii::$app->basePath . '/views/site/main.php');
$actual = $theme->applyTo(Yii::getApp()->basePath . '/views/site/main.php');

$this->assertPathEquals($expected, $actual);
}
Expand All @@ -148,7 +148,7 @@ public function testApplyToFilledPathMapAndInheritThemes()
$theme = new Theme($config);
$expected = Yii::getAlias('@app/framework/base/fixtures/themes/christmas/views/site/index.php');

$actual = $theme->applyTo(Yii::$app->basePath . '/views/site/index.php');
$actual = $theme->applyTo(Yii::getApp()->basePath . '/views/site/index.php');

$this->assertPathEquals($expected, $actual);
}
Expand All @@ -161,7 +161,7 @@ public function testApplyToFilledPathMapAndFileNotExists()
],
];
$theme = new Theme($config);
$expected = Yii::getAlias(Yii::$app->basePath . '/views/main/index.php');
$expected = Yii::getAlias(Yii::getApp()->basePath . '/views/main/index.php');

$actual = $theme->applyTo($expected);

Expand Down
6 changes: 3 additions & 3 deletions move-tests/web/helpers/HtmlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1386,13 +1386,13 @@ public function testActiveTextArea($value, array $options, $expectedHtml)
*/
public function testCsrfDisable()
{
Yii::$app->request->enableCsrfValidation = true;
Yii::$app->request->cookieValidationKey = 'foobar';
Yii::getApp()->request->enableCsrfValidation = true;
Yii::getApp()->request->cookieValidationKey = 'foobar';

$csrfForm = Html::beginForm('/index.php', 'post', ['id' => 'mycsrfform']);
$this->assertEquals(
'<form id="mycsrfform" action="/index.php" method="post">'
. "\n" . '<input type="hidden" name="_csrf" value="' . Yii::$app->request->getCsrfToken() . '">',
. "\n" . '<input type="hidden" name="_csrf" value="' . Yii::getApp()->request->getCsrfToken() . '">',
$csrfForm
);

Expand Down
2 changes: 1 addition & 1 deletion move-tests/web/helpers/JsonTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected function setUp()
{
parent::setUp();

// destroy application, Helper must work without Yii::$app
// destroy application, Helper must work without Yii::getApp()
$this->destroyApplication();
}

Expand Down
Loading

0 comments on commit cbe2365

Please sign in to comment.