Skip to content
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

Add new functions to disable "file tracking" & ease inline_css usage #91

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

weaverryan
Copy link
Member

@weaverryan weaverryan commented Apr 24, 2020

Hi!

Fixes #90

This contains several new features to help integration primarily with Mailer:

encore_disable_file_tracking()

{% do encore_disable_file_tracking() %}
    {{ encore_entry_link_tags('entry1') }}
    {{ encore_entry_script_tags('entry1') }}
{% do encore_enable_file_tracking() %}

This is for usage in Email or PDF templates to tell encore to "not" use its "file tracking" system temporarily. This avoids issues where, for example, if you rendered 2 emails at once, the 2nd would have no link tags because Encore things they are already rendered.

I also considered an alternative syntax - just having {% encore_encore_file_tracking %} one time at the top of the template. If people really like that, we can try that.

2) encore_entry_css_source()

This file has "tracking" automatically disabled.

{% apply inline_css(encore_entry_css_source('my_entry')) %}
    <div>
        Hi! The CSS from my_entry will be converted into
        inline styles on any HTML elements inside.
    </div>
{% endapply %}

I'd appreciate any feedback! Thanks!

Note to self: will simplify https://symfonycasts.com/screencast/mailer/pdf-styles and https://symfonycasts.com/screencast/mailer/encore-inline_css

@javiereguiluz
Copy link
Member

javiereguiluz commented May 8, 2020

Thanks for this contribution!

I'm a bit worried because of the added complexity and because the new feature is not obvious to discover.

I don't know if there are better names ... but "disable file tracking" it's a bit confusing because it's the indirect solution to the problem users are facing. If you have this issue, you'll Google for "encore assets disappear", "encore assets not available in the template", etc. The "file tracking" is the indirect cause of the problem ... that's why it's hard to associate both.

The general problem is that for "setter operations", this is very easy:

  • set() = set and replace existing
  • add() = set but not replace existing

For "getters operations":

  • get() = get and keep it (<-- this is the proposed encore_disable_file_tracking())
  • .....() = get and consume it (<-- this is the current behavior of Encore)

I don't know if there's a standard name in the industry for "get and consume". I asked in Symfony Slack and they proposed eat(), consume(), pop(), etc.

Comment on lines +30 to +70
yield 'no_overlap' => [
'/app/public',
'foo.js',
'/app/public/foo.js'
];

yield 'simple_overlap' => [
'/app/public/build',
'build/foo.js',
'/app/public/build/foo.js'
];

yield 'simple_overlap_with_slash' => [
'/app/public/build',
'/build/foo.js',
'/app/public/build/foo.js'
];

yield 'simple_overlap_with_build_path_slash' => [
'/app/public/build/',
'build/foo.js',
'/app/public/build/foo.js'
];

yield 'partial_overlap' => [
'/app/public/build',
'build/subdirectory/foo.js',
'/app/public/build/subdirectory/foo.js'
];

yield 'overlap_in_wrong_spot' => [
'/app/public/build',
'subdirectory/build/foo.js',
'/app/public/build/subdirectory/build/foo.js'
];

yield 'windows_paths' => [
'C:\\\\app\\public\\build',
'build/foo.js',
'C:\\\\app\\public\\build/build/foo.js'
];
Copy link
Contributor

@Kocal Kocal May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are files from dev-server supported?

Because that's an issue my team and me faced when working with Encore dev-server and assets in Twig for emails (with inky_to_html and inline_css)

We were able to make them work with a custom extension:

class EncoreExtension extends AbstractExtension
{
    private $publicDir;

    public function __construct(string $publicDir)
    {
        $this->publicDir = $publicDir;
    }

    public function getFunctions(): array
    {
        return [
            new TwigFunction('encore_source', [$this, 'source']),
        ];
    }

    public function source(string $path): string
    {
        // don't do anything if $path is http://my-app.domain:8008/...
        if (0 === strpos($path, '/')) {
            $path = $this->publicDir.$path;
        }

        // fetch content from filesystem or from http
        if (false !== $content = file_get_contents($path)) {
            return $content;
        }

        throw new \RuntimeException("Unable to read file \"$path\".");
    }
}
{% set css_to_inline = encore_entry_css_files('email') | map(file => encore_source(file)) | join('') %}
{% set css_to_ship = encore_entry_css_files('email-responsive') | map(file => encore_source(file)) | join('') %}

{% apply inky_to_html|inline_css(css_to_inline) %}
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width">

    <style>
    {{ css_to_ship }}
    </style>

    {# ... #}

But that would be really nice if it can be supported natively here :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really good point - I hadn't forgotten about the dev server. Assuming it did support the dev server (it actually does not currently), would the new encore_entry_css_source() Twig function work well for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give it a try!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated my code to this:

-{% set css_to_inline = encore_entry_css_files('email') | map(file => encore_source(file)) | join('') %}
-{% set css_to_ship = encore_entry_css_files('email-responsive') | map(file => encore_source(file)) | join('') %}
- 
-{% apply inky_to_html|inline_css(css_to_inline) %}
+{% apply inky_to_html|inline_css(encore_entry_css_source('email')) %}
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width">

    <style>
-    {{ css_to_ship }}
+    {{ encore_entry_css_source('email-responsive') }}
    </style>

    <style>

It works when running encore dev, but not when running the dev-server:

Capture d’écran de 2020-05-09 00-42-17

@weaverryan weaverryan changed the base branch from master to main November 16, 2020 19:53
@stof
Copy link
Member

stof commented Feb 16, 2021

Rather than a function to call in a do tag before and after (for which it is easy to forget undoing it after), I think this should be a tag instead (which will reset the state when reaching the end tag)

@Warxcell
Copy link
Contributor

Instead of manually handling this - I think it should be automatically done.

#115

@javiereguiluz
Copy link
Member

I think we should close this PR as "won't fix". We're focusing exclusively on AssetMapper, so we should no longer add new features to Webpack Encore. Thanks.

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

Successfully merging this pull request may close these issues.

Ability to "reset" cache of rendered files automatically in Twig
6 participants