Skip to content

Conversation

max-s-lab
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues

public function renderAjax($view, $params = [])
{
return $this->getView()->renderAjax($view, $params, $this);
return $this->view->renderAjax($view, $params, $this);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

As far as I know, $this->view is completely identical to $this->getView(), so nothing should break

Copy link
Member

Choose a reason for hiding this comment

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

Is it? 1. There's initialization happens. 2. One may extend and override the method.

Copy link
Contributor Author

@max-s-lab max-s-lab Sep 15, 2025

Choose a reason for hiding this comment

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

You're right. I fixed it using annotations

$method = [$model, $method];
} elseif ($method instanceof \Closure) {
$method = $this->method->bindTo($model);
$method = $method->bindTo($model);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link

codecov bot commented Sep 13, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.07%. Comparing base (04202f0) to head (e75e434).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
framework/web/Controller.php 0.00% 2 Missing ⚠️
framework/validators/InlineValidator.php 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (04202f0) and HEAD (e75e434). Click for more details.

HEAD has 19 uploads less than BASE
Flag BASE (04202f0) HEAD (e75e434)
21 2
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #20523       +/-   ##
=============================================
- Coverage     64.49%   21.07%   -43.42%     
+ Complexity    11574    11473      -101     
=============================================
  Files           433      433               
  Lines         37605    37519       -86     
=============================================
- Hits          24253     7907    -16346     
- Misses        13352    29612    +16260     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

}
} else {
$isClass = $class !== null && !$class->isBuiltin();
$isClass = $class instanceof \ReflectionNamedType && !$class->isBuiltin();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

This code is executed for PHP8.0 and above, so isBuiltin is only available in ReflectionNamedType. Nothing should break

image

Copy link
Member

Choose a reason for hiding this comment

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

Can you achieve the same with some annotations instead? Adding more checks, which aren't useful, to DI container costs performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


$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::$app->runAction('mime-type', ["$frameworkPath/helpers/mimeTypes.php"]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure it works after the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I checked it out.

image image

*
* @phpstan-var \yii\web\View&object{
* context: \yii\build\controllers\TranslationController,
* } $this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@max-s-lab max-s-lab marked this pull request as ready for review September 13, 2025 10:17

$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::$app->runAction('mime-type', ["$frameworkPath/helpers/mimeTypes.php"]);
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure it works after the change?

}
} else {
$isClass = $class !== null && !$class->isBuiltin();
$isClass = $class instanceof \ReflectionNamedType && !$class->isBuiltin();
Copy link
Member

Choose a reason for hiding this comment

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

Can you achieve the same with some annotations instead? Adding more checks, which aren't useful, to DI container costs performance.

public function renderAjax($view, $params = [])
{
return $this->getView()->renderAjax($view, $params, $this);
return $this->view->renderAjax($view, $params, $this);
Copy link
Member

Choose a reason for hiding this comment

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

Is it? 1. There's initialization happens. 2. One may extend and override the method.

@max-s-lab max-s-lab requested a review from samdark September 15, 2025 19:25
@samdark samdark merged commit 53bdfc3 into yiisoft:master Sep 15, 2025
25 of 96 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants