Skip to content

[DependencyInjection] when@{env} does not work on imported files #41584

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.

Sign up for GitHub

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

Jump to bottom
Closed
nusje2000 opened this issue Jun 7, 2021 · 4 comments · Fixed by #41597
Closed

[DependencyInjection] when@{env} does not work on imported files #41584

nusje2000 opened this issue Jun 7, 2021 · 4 comments · Fixed by #41597
Labels
Bug DependencyInjection Status: Needs Review

Comments

@nusje2000
Copy link
Contributor

Symfony version(s) affected: ^5.3.0

Description
When using the new feature for defining multiple environments in a single config files, you cannot import a file containing the when@{env} key. Although it is valid, it won't be used. Within the YamlFileLoader I found that when importing the files the environment property is temporary unset and is restored after finishing the import.

I don't know if this is an actual bug since it looks like this feature is not available in imported files on purpose.

How to reproduce
config.yml

imports:
    - { resource: profiler.yml }

profiler.yml

when@dev:
    web_profiler:
        toolbar: true

when@test:
    web_profiler:
        toolbar: false

Possible Solution
I looked at the YamlFileLoader and discovered that the environment was unset after loading the initial file (containing the import). I think it should be possible to remove this logic and have it work in imports too.
@nusje2000 nusje2000 added the Bug label Jun 7, 2021
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 7, 2021 •
edited
Loading

Can you please provide a reproducer or maybe a failing test case?

@nusje2000
Copy link
Contributor Author

Yes, I'm working on it 👍

@nusje2000
Copy link
Contributor Author

I've created a reproducer: https://github.com/nusje2000/symfony-41584-reproducer. See https://github.com/nusje2000/symfony-41584-reproducer/runs/2764838070 for the failing test result.

@nicolas-grekas I think I know how to fix this issue, so if you think this is a bug as well, I can open a PR.

@nicolas-grekas
Copy link
Member

Please do yes!

@nusje2000 nusje2000 mentioned this issue Jun 7, 2021
Merged
nicolas-grekas added a commit that referenced this issue Jun 8, 2021
@nicolas-grekas
bug #41597 [DependencyInjection] fix when@{env} inside imported fil…
c9cf773 
…es (nusje2000)

This PR was submitted for the 5.4 branch but it was squashed and merged into the 5.3 branch instead.

Discussion
----------

[DependencyInjection] fix `when@{env}` inside imported files

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| Tickets       | Fix #41584
| License       | MIT

This PR enables users to use the `when@{environment}` in imported files. I found that the env was explicitly unset when loading the file (including its imports) which resulted in the environment not being set when processing the imports.

See https://github.com/nusje2000/symfony-41584-reproducer for a reproduction of the problem.

Commits
-------

18fe0dd [DependencyInjection] Removed unsetting of environment property in loaders
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[DependencyInjection] fix when@{env} inside imported files nusje2000/symfony
3 participants
@nicolas-grekas @carsonbot @nusje2000