Symfony Flex Recipe

[robfrawley]

Q	A
Bug Report?	no
Feature Request?	yes
BC Break Report?	no
RFC?	no
Imagine Bundle Version	1.7.x

I've created an initial stab at a Symfony Flex contributor recipe, which I'll be submitting. Take a look at my fork of the contributor recipe repo and the added files to support this bundle, and let me know if we should add any additional items to it.

---

See symfony/recipes-contrib#14 for the pull request submitted against the contributed recipe repository.

[alexwilson]

Great idea, and the recipe you've submitted looks like a good starting point too (We don't want to overcomplicate/prematurely overconfigure if this is a potential entrypoint for new users!). I've posted a comment about the issue raised by the PR checker.

LGTM!

[Koc]

@robfrawley why copy-paste and not just include bundle routing file inside routing/imagine.yaml?

[robfrawley]

@Koc The original intention of using YAML in the Flex recipe was to lower the entry barrier for someone looking to, perhaps, edit the route paths; XML is generally more difficult for newcomers to follow or edit. Do we want to prioritize ease of use in the recipe or duplication removal?

I don't have a hard-fast answer to that question; let's solicit some input from everyone.

[robfrawley]

How does everyone feel about converting our XML to YAML (as proposed by Koc) so it can be easily used by the recipe in a user-friendly format, as well as used in the Flex recipe through the manifest.json file (instead of having two copies of it).

This question only applies to the 2.x branch as we obviously cannot remove the XML in 1.x and break everyone's prior usage. @Koc @antoligy Thoughts?

[robfrawley]

@Koc Perhaps we can combine a few of your ideas to resolve this?

Let's change imagine.xml to import our new imagine.yaml file on the 1.x branch, and then remove the imagine.xml file on the 2.x branch, giving us:

<!-- /Resources/config/routing.xml -->

<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://symfony.com/schema/routing
        http://symfony.com/schema/routing/routing-1.0.xsd">

    <import resource="@LiipImagineBundle/Resources/config/routing.yaml" />
</routes>

# /Resources/config/routing.yaml

liip_imagine_filter: 
    path: '/media/cache/resolve/{filter}/{path}'
    defaults:
        _controller: '%liip_imagine.controller.filter_action%'
    methods: 
        - GET
    requirements: 
        filter: '[A-z0-9_\-]*'
        path: .+

liip_imagine_filter_runtime: 
    path: '/media/cache/resolve/{filter}/rc/{hash}/{path}'
    defaults:
        _controller: '%liip_imagine.controller.filter_runtime_action%'
    methods: 
        - GET
    requirements: 
        filter: '[A-z0-9_\-]*'
        path: .+

This will allow us to immediately benefit from the change by using the manifest.json file to copy the routing from the package, instead of duplicating it in the recipe, while maintaining backward compatibility, and removing the backward compatibility layer from 2.x. Turning our manifest.json into the following in both 1.x and 2.x Flex recipes:

{
    "bundles": {
        "Liip\\ImagineBundle\\LiipImagineBundle": ["all"]
    },
    "copy-from-recipe": {
        "etc/": "%ETC_DIR%/"
    },
    "copy-from-package": {
        "Resources/config/routing.yaml": "%ETC_DIR%/routing/"
    }
}

[Koc]

@robfrawley awesome idea - with BC, thank you

[Pierstoval]

In your case, I suggest you to move your 2.x routing files into two files, as you have only two routes.

The goal here is to allow users to customize both routes prefix instead of copy/pasting code.

For example, in my OrbitaleCmsBundle recipe I just copy/paste the routing proposed by the doc, but actually, the page routing file for example is very light, and has no special routing prefixes in it so the user is free to have its own.

This is the most flexible way of doing things, so maybe this is a solution for your 2.x branch: remove the /media/cache/resolve part from your routing, and let the user add it with a prefix, warn him in the docs, and put this prefix by default in Flex.

What do you think?

EDIT: Bonus, I suggest you to move the liip_imagine_filter_runtime route before the liip_imagine_filter one, because as its base only differs with the /rc part (only for the base url). It allows the Symfony router to fasten checks over this route because it doesn't have to check for a {path} before checking for /rc.
Ok, I admit it, it might improve routing perfs by 0.01%, but still I think it's a good practice 🤣

[robfrawley]

@Pierstoval The ordering was an error on my part in the YAML variant and was fixed already when I ran the test suite against #925 (the XML was always ordered that way, I accidently reversed it). It's actually not just a performance issue, as the router requirement for path is .+, causing it to consume the rest of the URL and never match the second route when ordered that way. Thanks for the extra eyes though, in case I hadn't caught it; love the additional activity on this bundle!

As for your other implementation description, I'm not sure I quite follow what you are trying to achieve or how it should be implemented. Can you further clarify?

[Pierstoval]

Clarify about what part exactly?

[robfrawley]

Anyone chummy with a @symfony core member? We need either one of them or a @liip organization member to approve symfony/recipes-contrib#14 for it to be automatically merged by @symfony-bot. Perhaps @lsmith77 can help? Anyone know someone else? For the time being our PR will remain open and unmerged until we find someone to help with this. :-(

Update: we're all good with a merge from fabpot!