8000 [Framework] Add tag assets.package to register asset packages by GromNaN · Pull Request #38473 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Framework] Add tag assets.package to register asset packages #38473

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

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

GromNaN
Copy link
Member
@GromNaN GromNaN commented Oct 7, 2020

Replaces #38366

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR symfony/symfony-docs#14962

To configure asset packages in an application, we have to declare it in the framework configuration (doc for assets config). In some case we cannot use this configuration:

  • To use a custom class as package
  • To register an asset package in a shared bundle (my use-case).

This PR adds the assets.package tag. This tag is use to collect and inject package services into the assets.packages service, that is the registry for all packages. Since every package needs a name, the package attribute of the tag is used (same naming convention that the console.command tag).

Main changes:

  • the packages defined in the framework.assets configuration are injected into the assets.packages using the tag instead of being directly injected in service declaration.
  • changed signature of Symfony\Components\Assets\Packages constructor to accept an iterator (backward compatible).
  • a new alias assets._default_package is defined even if assets are not configured.

Example in symfony/demo (commit):

In config/services.yaml:

    avatar.strategy:
        class: Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy
        arg
8000
uments:
            - '%kernel.project_dir%/public/build/manifest.json'

    avatar.package:
        class:  Symfony\Component\Asset\Package
        arguments:
            - '@avatar.strategy'
            - '@assets.context'
        tags:
            - { name: assets.package, package: avatars }

Then we can use the package anywhere

<img src="{{ asset('anna.jpg', 'avatars') }}">

Alternative using autoconfiguration with a custom class:

With a custom class implementing the PackageInterface, the package name can be provided by a the static method getDefaultPackageName. Autowiring and autoconfiguration will import the package.

namespace App\Asset;

use Symfony\Component\Asset\PackageInterface;

class AvatarPackage implements PackageInterface
{
    public static function getDefaultPackageName(): string
    {
        return 'avatars';
    }

    // ... Implements the interface
}

@Nyholm
Copy link
Member
Nyholm commented Oct 22, 2020

Thank you for this PR.

I like it, it is a rare feature but I think we should support it.

@nicolas-grekas
Copy link
Member

(rebase needed)

@GromNaN
Copy link
Member Author
GromNaN commented Jan 26, 2021

Thank you for the reminder. The branch has been rebased.

Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks (with minor comments)

Autoconfiguration with PackageInterface
@fabpot
Copy link
Member
fabpot commented Feb 16, 2021

Thank you @GromNaN.

@fabpot fabpot merged commit 1262b06 into symfony:5.x Feb 16, 2021
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 19, 2021
This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Framework] Add tag assets.package

Documentation for symfony/symfony#38473

Commits
-------

f4dbec4 [Framework] Add tag assets.package
@fabpot fabpot mentioned this pull request Apr 18, 2021
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.

6 participants
0