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

Explain why DI fails sometimes and how to fix this (#20010) #20108

Conversation

skepticspriggan
Copy link
Contributor

Q A
Is bugfix?
New feature?
Breaks BC?
Fixed issues #20010

Copy link

what-the-diff bot commented Jan 27, 2024

PR Summary

  • Enhancement to Yii's Core Code
    This update provides additional guidance on how to enhance Yii's core functionality with dependency injection mechanisms, specifically within the yii\db\ActiveRecord model. This will allow developers more flexibility and control over dependencies in their Yii projects.

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0027227) 48.02% compared to head (166a4ad) 48.02%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #20108   +/-   ##
=======================================
  Coverage   48.02%   48.02%           
=======================================
  Files         445      445           
  Lines       43892    43892           
=======================================
  Hits        21080    21080           
  Misses      22812    22812           

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

@@ -396,6 +396,18 @@ class HotelController extends Controller
}
```

Note that not all core code supports DI due to performance concerns. You can add support if needed by overriding
Copy link
Member

Choose a reason for hiding this comment

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

I think that's correct for AR only. Are there any other cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not find other cases.

I wrote it this way because it seemed the most durable. It is technically correct now and would also be true if a case is discovered or added in the future.

Copy link
Member

Choose a reason for hiding this comment

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

I'd move this note into AR guide instead.

@samdark samdark merged commit c2e0485 into yiisoft:master Feb 16, 2024
67 checks passed
@samdark
Copy link
Member

samdark commented Feb 16, 2024

Thanks.

@skepticspriggan
Copy link
Contributor Author

Glad to contribute

@skepticspriggan skepticspriggan deleted the 20010-explain-why-di-fails-sometimes-and-how-to-fix-this branch February 19, 2024 12:05
skepticspriggan added a commit to skepticspriggan/yii2 that referenced this pull request Mar 23, 2024
…iisoft#20108)

* Explain why DI fails sometimes and how to fix this (yiisoft#20010)

* Explain why AR does not support DI by default and how to support it (yiisoft#20010)
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