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

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

Open
wants to merge 1 commit into
base: master
from

Conversation

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 24, 2020

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 nicolas-grekas:di-preload-tag branch 2 times, most recently from 82543d0 to f15656e Mar 25, 2020
@michaljusiega

This comment has been minimized.

Copy link
Contributor

michaljusiega commented Mar 25, 2020

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

@stof

This comment has been minimized.

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 nicolas-grekas force-pushed the nicolas-grekas:di-preload-tag branch from f15656e to 45dd84f Mar 25, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Mar 25, 2020

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 nicolas-grekas:di-preload-tag branch 2 times, most recently from b62ee86 to b075b40 Mar 25, 2020
@michaljusiega

This comment has been minimized.

Copy link
Contributor

michaljusiega commented Mar 25, 2020

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

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:di-preload-tag branch from b075b40 to 494338a Mar 31, 2020
…ses to preload/services to not preload
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:di-preload-tag branch from 494338a to c0121bf Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.