liip / LiipImagineBundle

Symfony Bundle to assist in image manipulation using the imagine library
http://liip.ch
MIT License
1.66k stars 378 forks source link

Add support for JsonManifestVersionStrategy #1525

Closed wouterSkepp closed 1 year ago

wouterSkepp commented 1 year ago
Q A
Branch? 2.x
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets #1524
License MIT
Doc

This PR adds in support for JsonManifestVersionStrategy that is available in Symfony. In a JSON manifest, we have a key => value structure where the key is the original filepath, and the value the versioned filestring, e.g:

{
  "build/images/cats.jpg": "/build/images/cats.jpg?v=dda78c62c1f2a8793334"
  "build/images/dogs.jpg": "/build/images/dogs.e6fbd6606990f549c912.jpg"
}

Similar to StaticVersionStrategy that loads it's configuration from the container, this reads the JSON Manifest file defined


framework:
    assets:
        json_manifest_path: '%kernel.project_dir%/public/build/manifest.json'

The LazyFilterRuntime has been extended to load the JSON Manifest file contents, and the various methods within to handle both Static & Json versioning.

Note that the current 2.x branch has unrelated failing CI.

I don't think this PR breaks current implementations although test coverage for AssetsVersionCompilerPass currently is lacking.

coveralls commented 1 year ago

Coverage Status

coverage: 81.851%. first build when pulling eb3132fc88a3cf829ae9b2fa6ec2a2c07ffcb967 on skepp:json_manifest_version_strategy_support into 3c0f20b42d2ea1816ffa4e606c8249c229edc450 on liip:2.x.

dbu commented 1 year ago

i will fix the cs failures in the 2.x branch to be clean there.

dbu commented 1 year ago

i will fix the cs failures in the 2.x branch to be clean there.

the 2.x branch is green again. please rebase your branch on 2.x

wouterSkepp commented 1 year ago

Thank you for your helpful review! Updated PR accordingly.

i am not super sure if the path / resolvedPath are always the file path or if they can be the cache path as well.

I've not done a deep dive into the caching mechanism, but according to https://github.com/liip/LiipImagineBundle/blob/2.x/Resources/doc/asset-versioning.rst?plain=1#L10-L19 and from what I've seen so far is that this is always the original filepath.

dbu commented 1 year ago

i rebased and squashed the commits in #1529