Added LazyAsseticNode which can be modified by NodeVisitors #574

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

Twig allows to modify template nodes in node visitors in order to influence the way they are generated. With the current implementation of AsseticNode however, the possibilities are limited, because the assets are already created when the node is created.

I added a separate LazyAsseticNode which removes this limitation. There, the asset is only created during compile time (unless it has been created by a visitor already). In this way, you can write node visitors which modify the node's inputs, filters, name etc. before creating the asset:

class MyNodeVisitor implements \Twig_NodeVisitor
{
    protected function enterNode(\Twig_NodeInterface $node)
    {
        if ($node instanceof LazyAsseticNode) {
            $inputs = $node->getAttribute('inputs');

            foreach ($inputs as $key => $value) {
                $inputs[$key] = // do stuff ...
            }

            $node->setAttribute('inputs', $inputs);
        }
    }
}

Lazy asset creation can be activated by passing true to the fourth argument $lazyAssets of the AsseticExtension. This way, the implementation is fully backwards compatible.

An alternative solution would be to change the existing AsseticNode/AsseticTokenParser to create the asset on-demand. A consequence would be that the "asset" attribute of the node would not be available anymore during the traversal of the node tree. Any node visitor that currently uses this attribute would break.

If you prefer the second solution regardless of the BC break, I can adapt the PR.

Collaborator

stof commented Feb 25, 2014

I see another way to do the implementation, allowing to have the benefits of both implementations at once: moving the creation of assets to a node visitor. This way, a node visitor willing to change the attributes could do it by using a higher priority, and a node visitor willing to access the asset could use a lower priority to run after the creation. The drawback is that this would only apply to either leaveNode or enterNode (depending of the implementation of the visitor) but not both.

I'm not sure if this would make sense though. It is just a quick idea.

Anyway, I don't really like the toggle between 2 different node classes based on the flag. This would create weird issues if you have 1 library relying on the old behavior and another one relying on the new behavior, as they would not be able to run together (and understanding why they don't work properly could be really hard).
Thus, given that Twig node visitor are totally undocumented, I'm not sure they are used by lots of people to modify the assetic nodes. So a BC break here could be fine (but maybe @kriswallsmith has a few other BC breaking ideas which could go in the same major release then)

I agree that the toggle is ugly and I would happily remove it. I just wanted to make sure that's not against your intentions.

The idea with the node visitor is a good one as well. If @kriswallsmith agrees, I can change the implementation accordingly.

Any chance for an update? Without this, I can't get the Puli integration for Assetic to work.

Owner

kriswallsmith commented Nov 17, 2014

Apologies for going dark on this. Could you add a setCurrentDirectory() method to the Puli asset classes and call that from the visitor? If you move resolution logic down into the asset class, that should work, no?

I think this can safely be closed now. After talking to Kris, I used a different approach in https://github.com/puli/assetic-extension.

webmozart closed this Mar 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment