Skip to content

Conversation

@GromNaN
Copy link
Member

@GromNaN GromNaN commented Sep 30, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR symfony/symfony-docs#14313

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
        arguments:
            - '%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('build/images/fa-brands-400.svg', 'avatars') }}">

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.

Should we register the new tag for autoconfiguration?

@GromNaN
Copy link
Member Author

GromNaN commented Oct 1, 2020

Should we register the new tag for autoconfiguration?

We need the name of the package. This property is not provided by the PackageInterface. I tried an implementation in GromNaN#1, but this leads to some breaking changes in PackageInterface and all the classes that uses it.

@GromNaN GromNaN requested a review from nicolas-grekas October 1, 2020 20:26
@GromNaN GromNaN force-pushed the assets-package-tag branch from c6d2684 to e854986 Compare October 6, 2020 21:26
@GromNaN
Copy link
Member Author

GromNaN commented Oct 6, 2020

I figured out that there are 3 ways to name the package when a service is tagged (code in PriorityTaggedServiceTrait)

1. Explicit name in tag definition

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

The package get its name from the tag definition: avatars.
Usage:

<img src="{{ asset('style.css', 'avatars') }}">

2. Using a static method of the "custom" class

When using a custom class, the package name is statically returned by the method getDefaultName of that class (like for console commands).

    App\Asset\AvatarPackage:
        arguments:
            - '@avatar.strategy'
        tags:
            - { name: assets.package }
namespace App\Asset;

use Symfony\Component\Asset\Package;

class AvatarPackage extends Package
{
    public static function getDefaultName(): string
    {
        return 'avatars';
    }
}

Usage:

<img src="{{ asset('style.css', 'avatars') }}">

2. Fallback to the service Id

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

When the tag does not have the attribute package and the method getDefaultName is not defined, the package gets the name of the service id avatar.package.

Usage:

<img src="{{ asset('style.css', 'avatar.package') }}">

Conclusion

I think, this is an acceptable solution to setup autoconfiguration of the tag for PackageInterface ; and that needs some documentation.

@fabpot fabpot closed this Oct 7, 2020
@fabpot
Copy link
Member

fabpot commented Oct 7, 2020

We've just moved away from master as the main branch to use 5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to 5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

@nicolas-grekas nicolas-grekas modified the milestones: 5.x, 5.2 Oct 14, 2020
fabpot added a commit that referenced this pull request Feb 16, 2021
…ackages (GromNaN)

This PR was merged into the 5.3-dev branch.

Discussion
----------

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

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](https://symfony.com/doc/current/reference/configuration/framework.html#assets)). 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](symfony/demo@e5e5a8f...GromNaN:assets-package-tag)):

In `config/services.yaml`:
```yaml
    avatar.strategy:
        class: Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy
        arguments:
            - '%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
```twig
<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.

```php
namespace App\Asset;

use Symfony\Component\Asset\PackageInterface;

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

    // ... Implements the interface
}
```

Commits
-------

6217ff7 [Asset] Add tag assets.package to register asset packages
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.

4 participants