Skip to content

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Sep 26, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? no
Deprecations? yes
Issues -
License MIT

The target of this change is being able to patch the PHP-DSL examples in the documentation without any downsides:

-return static function (FrameworkConfig $framework) {
+return function (FrameworkConfig $framework) {

@carsonbot carsonbot added this to the 8.0 milestone Sep 26, 2025
@carsonbot carsonbot changed the title [DependencyInjection][Config][Routing] Close access to the internal scope of the loader when using PHP files for configuration [Config][DependencyInjection][Routing] Close access to the internal scope of the loader when using PHP files for configuration Sep 26, 2025
@stof
Copy link
Member

stof commented Sep 26, 2025

the changelog mentions using the $loader variable as a replacement, but there is no such example neither in the PR description nor in the upgrade file.

@nicolas-grekas
Copy link
Member Author

yeah, I think this is too niche anyway

@nicolas-grekas nicolas-grekas changed the base branch from 8.0 to 7.4 September 29, 2025 15:21
@nicolas-grekas nicolas-grekas changed the title [Config][DependencyInjection][Routing] Close access to the internal scope of the loader when using PHP files for configuration [Config][DependencyInjection][Routing] Deprecate using $this and its internal scope from PHP config files; use the $loader variable instead Sep 29, 2025
@nicolas-grekas nicolas-grekas changed the title [Config][DependencyInjection][Routing] Deprecate using $this and its internal scope from PHP config files; use the $loader variable instead [Config][DependencyInjection][Routing] Deprecate using $this or the internal scope of the loader from PHP config files Sep 29, 2025
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 29, 2025

PR updated, I figured out a BC layer!

Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

I suggest adding a test covering this BC layer (as the SecurityBundle functional tests have been migrated to stop using this feature)

@nicolas-grekas nicolas-grekas force-pushed the di-php-static-loader branch 2 times, most recently from 05702a1 to 3ab95d2 Compare September 29, 2025 17:20
@nicolas-grekas
Copy link
Member Author

Tests added

… internal scope of the loader from PHP config files
@nicolas-grekas nicolas-grekas removed this from the 8.0 milestone Sep 29, 2025
@nicolas-grekas nicolas-grekas added this to the 7.4 milestone Sep 29, 2025
@nicolas-grekas nicolas-grekas merged commit 172e24c into symfony:7.4 Sep 29, 2025
11 of 12 checks passed
@nicolas-grekas nicolas-grekas deleted the di-php-static-loader branch October 2, 2025 08:13
This was referenced Oct 27, 2025
nicolas-grekas added a commit that referenced this pull request Dec 4, 2025
…rnal scope from PHP config files (nicolas-grekas)

This PR was merged into the 7.4 branch.

Discussion
----------

[DependencyInjection] Throw when using `$this` or its internal scope from PHP config files

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | #62547
| License       | MIT

The BC layer brought by #61860 doesn't work: as described in #62547, the first call to load() has side effects which can break when the second and fallback call is made.

I don't see any other way. Well, one would be to operate on a clone, and if that works, rerun with the real container builder. But the overhead is going to be high for everybody.

I'm therefor proposing to change this as a BC BREAK. While less than ideal, this happens at compile time, and the fix is trivial and explained in the error message.

Commits
-------

7ffcd64 [DependencyInjection] Throw when using `$this` or its internal scope from PHP config files; use the `$loader` variable instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants