twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.18k stars 1.25k forks source link

ChainLoader constructor should accept iterable instead of array #4212

Closed TheCelavi closed 3 months ago

TheCelavi commented 3 months ago

Closes #4200

stof commented 3 months ago

ah indeed. Supporting lazy-loaded iterators would require making the ChainLoader configuration immutable instead of having this addLoader. So let's forget it...

TheCelavi commented 3 months ago

There is also this method which can be used to preserve lazy loading:

    public function addLoader(LoaderInterface $loader): void
    {
        $existing = $this->loaders;

        $this->loaders = static function () use ($existing, $loader): \Generator {
            yield from $existing;
            yield $loader;
        };

        $this->hasSourceCache = [];
    }

I don't think that it would have much of the performance impact when adding 5-10 loaders (hardly real-world scenario, people usually use 1-2 loaders and that is it).

Are you more interested in this method? @stof

xabbuh commented 3 months ago

We could deprecate addLoader() in 3.x and introduce lazy loading in Twig 4.

TheCelavi commented 3 months ago

I have updated PR with modification which will support preserving lazy loading of loaders with addLoaders() method.

fabpot commented 3 months ago

Thank you @TheCelavi.