10000 [DI] add tags `container.preload`/`.no_preload` to declare extra classes to preload/services to not preload by nicolas-grekas · Pull Request #36195 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] add tags container.preload/.no_preload to declare extra classes to preload/services to not preload #36195

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
Merged
merged 1 commit into from
Apr 5, 2020
Merged

[DI] add tags container.preload/.no_preload to declare extra classes to preload/services to not preload #36195

merged 1 commit into from
Apr 5, 2020

Conversation

nicolas-grekas
Copy link
Member
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

To allow fine-grained declaration of sidekick classes in DI extensions.
Follows #36103

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 24, 2020
@nicolas-grekas nicolas-grekas changed the title [DI] add tag "container.preload" tag to declare sidekick classes to preload [DI] add tag "container.preload" to declare sidekick classes to preload Mar 25, 2020
@nicolas-grekas nicolas-grekas force-pushed the di-preload-tag branch 2 times, most recently from 82543d0 to f15656e Compare March 25, 2020 08:34
@michaljusiega
Copy link
Contributor

So class_exists is no longer needed in favor of container.preload tag do add classes to *.preload.php dumped file ?

@stof
Copy link
Member
stof commented Mar 25, 2020

@michaljusiega AFAIU, the class_exists were solving 2 issues:

  • discovery of classes to be preloaded
  • compatibility with preloading is some specific cases related to anonymous classes which break if the class is not loaded

This PR provides an alternative solution to the first issue (though class_exists will still work). For the second problem, class_exists is still the proper solution IMO, as that's not about the Symfony preload dumper by itself, but about compat with preloading.

@nicolas-grekas nicolas-grekas changed the title [DI] add tag "container.preload" to declare sidekick classes to preload [DI] add tags container.preload/.no_preload to declare extra classes to preload/services to not preload Mar 25, 2020
@nicolas-grekas
Copy link
Member Author
nicolas-grekas commented Mar 25, 2020 •
edited
Loading

I updated the PR to now also provide a container.no_preload tag.
I added it to eg. console commands.

@michaljusiega: what @stof said: class_exists() are better options for portability and for locality. container.preload serves adding preloading info to third party packages that don't (want to) use class_exists(), and also in programmatic situations where a bundle's logic might known better about fine grained preloading, depending on the configuration.

@nicolas-grekas nicolas-grekas force-pushed the di-preload-tag branch 2 times, most recently from b62ee86 to b075b40 Compare March 25, 2020 17:46
@michaljusiega
Copy link
Contributor

@nicolas-grekas, @stof I understand. Thanks!

@nicolas-grekas nicolas-grekas force-pushed the di-preload-tag branch 3 times, most recently from c0121bf to 966ad2d Compare April 4, 2020 09:54
@fabpot
Copy link
Member
fabpot commented Apr 5, 2020

Thank you @nicolas-grekas.

@fabpot fabpot mentioned this pull request Apr 5, 2020
Closed
@fabpot fabpot merged commit 3b38f38 into symfony:master Apr 5, 2020
@nicolas-grekas nicolas-grekas deleted the di-preload-tag branch April 5, 2020 16:59
@nicolas-grekas nicolas-grekas removed this from the next milestone May 4, 2020
@nicolas-grekas nicolas-grekas added this to the 5.1 milestone May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
@olsavmic olsavmic mentioned this pull request Aug 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosier rosier rosier left review comments

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
DependencyInjection Feature Ready Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
6 participants
@nicolas-grekas @michaljusiega @stof @fabpot @rosier @carsonbot
0