Skip to content

encore_entry_css_files returns nothing if called multiple time #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
soullivaneuh opened this issue Jan 21, 2019 · 12 comments
Closed

encore_entry_css_files returns nothing if called multiple time #33

soullivaneuh opened this issue Jan 21, 2019 · 12 comments

Comments

@soullivaneuh
Copy link

I'm using this function to render PDF with inline CSS as it was suggested in #16 (comment):

<style>{{ asset_get_content(encore_entry_css_files('pdf')) }}</style>

But if I call the twig render method multiple time (to render multiple PDFs on one command), only the first will get CSS.

For the other, the css file list is just empty.

So I tried this for test on my homepage:

{{ dump(encore_entry_css_files('pdf')) }}
{{ dump(encore_entry_css_files('pdf')) }}
{{ dump(encore_entry_css_files('pdf')) }}

And it confirms the issue:

image

To me, it's a bug. Why this function would not return the needed files if call multiple times?

@soullivaneuh
Copy link
Author

Ok I found why:

// make sure to not return the same file multiple times
$entryFiles = $entryData[$key];
$newFiles = array_values(array_diff($entryFiles, $this->returnedFiles));
$this->returnedFiles = array_merge($this->returnedFiles, $newFiles);

There is certainly a reason, maybe for tag rendering?

But for that use case, it should have an option to force duplicate returns IMO.

@soullivaneuh
Copy link
Author

soullivaneuh commented Jan 21, 2019

The workaround to this is to inject the entrypoint lookup and reset it before each twig render call.

See my complete service:

final class PdfRenderer
{
    /**
     * @var EngineInterface
     */
    private $twig;

    /**
     * @var Dompdf
     */
    private $pdf;

    /**
     * @var TranslatorInterface&LocaleAwareInterface
     */
    private $translator;

    /**
     * @var EntrypointLookupInterface
     */
    private $encoreEntrypointLookup;

    public function __construct(EngineInterface $twig, Dompdf $pdf, TranslatorInterface $translator, EntrypointLookupInterface $encoreEntrypointLookup)
    {
        Assert::isInstanceOf($translator, LocaleAwareInterface::class);
        $this->twig = $twig;
        $this->pdf = $pdf;
        $this->translator = $translator;
        $this->encoreEntrypointLookup = $encoreEntrypointLookup;
    }

    /**
     * Generates the HTML code with Twig and pushes it to the PDF generator.
     *
     * @param mixed[] $parameters
     */
    public function renderPdf(string $template, array $parameters, ?string $locale = null): string
    {
        // We have to get a new Dompdf instance to avoid this issue: https://github.com/dompdf/dompdf/issues/1056
        // We will be able to fallback to the properties once resolved and released.
        // You can test it again using the nexy:invoice:remind command. ATM, it fails generating multiple reminders.
        $pdf = new Dompdf($this->pdf->getOptions());

        $fallbackLocale = $this->translator->getLocale();

        if (null !== $locale) {
            $this->translator->setLocale($locale);
        }

        $this->encoreEntrypointLookup->reset();
        $pdf->loadHtml($this->twig->render($template, $parameters));
        $pdf->render();
        $this->translator->setLocale($fallbackLocale);

        return (string) $pdf->output();
    }
}

@eschultz-magix
Copy link

Same as #31 and you need to wait for #21 to complete.

@MrMitch
Copy link

MrMitch commented Feb 1, 2019

@soullivaneuh how did you get the EntrypointLookupInterface to be injected in your service ?

Only adding the parameter to the constructor throws an exception

Cannot autowire service "MyClass": argument "$encoreEntryPointLookup" of 
method "MyClass::__construct()" references interface 
"Symfony\WebpackEncoreBundle\Asset\EntrypointLookupInterface" 
but no such service exists. 
You should maybe alias this interface to one of these existing services: 
"webpack_encore.entrypoint_lookup", "webpack_encore.entrypoint_lookup[_default]".

As per de recommendation in the exception message, I added an alias but that doesn't seem to work, it seems the container just tries to create a new EntrypointLookupInterface instance (with the wrong parameters) instead of using the existing webpack_encore.entrypoint_lookup service.

services:
    _defaults:
        autowire: true
        autoconfigure: true
        public: false

    Symfony\WebpackEncoreBundle\Asset\EntrypointLookupInterface:
        alias: 'webpack_encore.entrypoint_lookup'
Type error: Argument 1 passed to 
Symfony\WebpackEncoreBundle\Asset\EntrypointLookup::__construct() 
must be of the type string, object given, called in 
<project-root>/var/cache/dev/ContainerJpxofud/getWebpackEncore_EntrypointLookupService.php
on line 12

How did you get this to work ?

@MrMitch
Copy link

MrMitch commented Feb 1, 2019

It seems aliasing webpack_encore.entrypoint_lookup[_default] works and aliasing webpack_encore.entrypoint_lookup doesn't. (#36)

So with

services:
    _defaults:
        autowire: true
        autoconfigure: true
        public: false

    Symfony\WebpackEncoreBundle\Asset\EntrypointLookupInterface:
        alias: 'webpack_encore.entrypoint_lookup[_default]' # this is the important line

The injection of EntrypointLookupInterface into a service works fine.

@weaverryan
Copy link
Member

See #45 - I think reset() is probably what should be used for this, but autowiring should be easier.

@DennisdeBest
Copy link

Thanks !! this helped out a lot :)

@theredled
Copy link

Is it supposed to be a wanted behaviour ?

@Kocal
Copy link
Member

Kocal commented Nov 17, 2020

Yes it is.

@theredled
Copy link

Why ?

@Kocal
Copy link
Member

Kocal commented Nov 17, 2020

I don't know, sureley to prevent importing the same files multiples times which can lead to undesirable behaviour.
See #45, it add a reset method on the EntrypointLookupInterface and it may helps you.

Also, it will probably be usefull to have a way to reset the entrypoint lookup directly on Twig. WDYT @weaverryan?

@theredled
Copy link

I'm sorry but I don't understand how you can be sure it's a 100% wanted behaviour without knowing the reason why ?
I mean yes, including multiple times can lead to bugs just like any mistake... But how is that useful to prevent that one mistake ? Why not an error message ?

There is probably a better reason (or it's an issue dressed as a feature)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants