Moving `{% block importmap %}` *above* `{% block javascripts %}`

ThomasLandauer commented 8 months ago

Q	A
License	MIT
Doc issue/PR	none

@weaverryan Disclaimer: I'm not sure about this - especially I didn't check if before_target is actually doing what I want ;-)

Here's the scenario I'm talking about:

{# base.html.twig #}
{% block javascripts %}
    {% block importmap %}{{ importmap('app') }}{% endblock %}
    // Here's some important JavaScript stuff that's needed on *every* page
{% endblock %}

{# some_page.html.twig #}
{% block importmap %}{{ importmap(['app', 'some_page']) }}{% endblock %}

Now how can I get the other important JavaScript onto some_page? IMO only by doing:

{% block javascripts %}
    {{ parent() }}
{% endblock %}

But this will also render the importmap twice. Right?

My solution: Move {% block importmap %} outside of {% block javascripts %}. What do you think?

If you merge this, the code block at https://symfony.com/doc/6.4/frontend/asset_mapper.html#installation would need to be updated again... :-)

github-actions[bot] commented 8 months ago

Thanks for the PR 😍

How to test these changes in your application

Define the SYMFONY_ENDPOINT environment variable:

# On Unix-like (BSD, Linux and macOS)
export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1288/index.json
# On Windows
SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1288/index.json

Install the package(s) related to this recipe:

composer req 'symfony/flex:^1.16'
composer req 'symfony/asset-mapper:^6.4'

Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

# On Unix-like (BSD, Linux and macOS)
unset SYMFONY_ENDPOINT
# On Windows
SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes. I'm going keep this comment up to date with any updates of the attached patch.

symfony/asset-mapper

6.3 vs 6.4

```diff diff --git a/symfony/asset-mapper/6.3/assets/app.js b/symfony/asset-mapper/6.4/assets/app.js index cb0082a..6174cc6 100644 --- a/symfony/asset-mapper/6.3/assets/app.js +++ b/symfony/asset-mapper/6.4/assets/app.js @@ -4,4 +4,6 @@ * This file will be included onto the page via the importmap() Twig function, * which should already be in your base.html.twig. */ -console.log('This log comes from assets/app.js - welcome to AssetMapper! 🎉') +import './styles/app.css'; + +console.log('This log comes from assets/app.js - welcome to AssetMapper! 🎉'); diff --git a/symfony/asset-mapper/6.3/importmap.php b/symfony/asset-mapper/6.4/importmap.php index 5c2c21d..7e330f7 100644 --- a/symfony/asset-mapper/6.3/importmap.php +++ b/symfony/asset-mapper/6.4/importmap.php @@ -1,13 +1,13 @@ [ - 'path' => 'app.js', - 'preload' => true, + 'path' => './assets/app.js', + 'entrypoint' => true, ], ]; diff --git a/symfony/asset-mapper/6.3/manifest.json b/symfony/asset-mapper/6.4/manifest.json index c6fb477..1bbdf0c 100644 --- a/symfony/asset-mapper/6.3/manifest.json +++ b/symfony/asset-mapper/6.4/manifest.json @@ -6,22 +6,19 @@ }, "aliases": ["asset-mapper", "importmap"], "gitignore": [ - "/%PUBLIC_DIR%/assets/" + "/%PUBLIC_DIR%/assets/", + "/assets/vendor/" ], + "composer-scripts": { + "importmap:install": "symfony-cmd" + }, "add-lines": [ { "file": "templates/base.html.twig", - "content": " {{ importmap() }}", - "position": "after_target", + "content": "{% block importmap %}{{ importmap('app') }}{% endblock %}", + "position": "before_target", "target": "{% block javascripts %}", "warn_if_missing": true - }, - { - "file": "templates/base.html.twig", - "content": " ", - "position": "after_target", - "target": "{% block stylesheets %}", - "warn_if_missing": true } ], "conflict": { ```

smnandre commented 8 months ago

TL;DR; there is nothing to change

(and, personal opinion, i think this use case is a bit too specific to change the recipe anyway)

Base scenario

{# foo.html.twig #}
{% block javascripts %}

{% block importmap %} -importmap- {% endblock %}

--important js--

{% endblock %}

{# bar.html.twig #}
{% extends 'foo.html.twig' %}

Result:

-import map 
--important js--

Override importmap:

{# bar.html.twig #}
{% extends 'foo.html.twig' %}

{% block importmap %} -another import map {% endblock %}

Result:

  -another import map 
--important js--

Override important JS:

{# bar.html.twig #}
{% extends 'foo.html.twig' %}

{% block javascripts %} --another important js-- {% endblock %}

Result:

--another important js--

Override important JS + parent

{# bar.html.twig #}
{% extends 'foo.html.twig' %}

{% block javascripts %}
{{ parent() }}
 --another important js-- 
{% endblock %}

Result:

    -importmap- 
--important js--

    --another important js--

Override both:

{# bar.html.twig #}
{% extends 'foo.html.twig' %}

{% block importmap %}
- another importmap
{% endblock %}

{% block javascripts %}
{{ parent() }}
 --another important js-- 
{% endblock %}

Result:

- another importmap

--important js--

    --another important js--

https://twigfiddle.com/ke86dz

ThomasLandauer commented 7 months ago

Thanks, you're right! I thought it's required/recommended to override the blocks in the same nesting situation as they are defined:

{% block javascripts %}
{% block importmap %} - another importmap- {% endblock %}
{{ parent() }}
 --another important js-- 
{% endblock %}

Which then lead to my problem above.

So I'm closing here, since this is a general Twig misunderstanding, unrelated to this recipe.

Anyway, I still can't see any real reason why {% block importmap %} should live inside {% block javascripts %} - so I guess it's just a matter of personal taste.

symfony / recipes