This was a request from @bnjmnm during a meeting today to discuss committing SDC to core.

We want to investigate augmenting usage of include/embed with custom Twig function. This would have the benefit of not passing in context (similar to Twig's only keyword).

I think this leaves us with a custom function as our only option if we want to do this. However, I am increasingly convinced that we might now want to do this. The reason being documentation of the new method, and catching up with new features and bugfixes.

Comments

mherchel created an issue. See original summary.

e0ipso’s picture

Pros Cons
  • Allows us to enforce only passing the provided context. See docs about with_context. Which is desirable for components.
  • A function doesn't allow use of {% block foo %} which complicates support for slots.
  • We will need to document, and maintain documentation, of the new function
  • Developers new to Drupal might be already familiar with Twig standard functions include/embed
jonathanshaw’s picture

I think it should be possible to enforce with_context to be false when using include/embed with a component, using a Twig NodeVisitor.

For those innocent souls who have not yet encountered the unholy mystery that is Twig's NodeVisitors, see lib/Drupal/Core/Template/TwigNodeVisitor.php for an example of how Drupal is already altering the functionality of Twig's provided functions.

jonathanshaw’s picture

Title: SDC: Investigate augmenting usage of include/embed with custom Twig function » SDC: Prevent unwanted context passing with Twig include/embed functions

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

e0ipso’s picture

e0ipso’s picture

Another idea is to add a warning during development (in a similar fashion we use for validation errors). This warning could show a message in the page whenever a component is put in the page without the only keyword. This check should probably happen in the same node visitor we use for attaching the library to the page, like @jonathanshaw suggests.

e0ipso’s picture

Issue summary: View changes

Doing this in the include and embed will be a backwards compatibility break. Since SDC is beta experimental, we need to maintain BC. Additionally, this would be a departure from the expectations that include and embed work the same way as described in Twig's documentation.

jonathanshaw’s picture

As a version of #7, it should be possible to detect and warn in a node visitor if a variable that is not a defined slot/prop is used inside a component.

We now do something very similar to this to detect usage of deprecated variables, allowing us to deprecate variables passed to twig templates.

This would allow us to continue to use include/embed, with the documented behavior developers expect.

dieterholvoet’s picture

Component: theme system » single directory components
brayn7’s picture

I have encountered this issue in tandem with https://www.drupal.org/project/drupal/issues/3464719. If the only keyword is forced that would be good but we gotta make sure blocks from embed still get passed down and not blocked. Let me know if I need to post an example. I find myself needing to pass down a block for an embed to a nested embed block basically. Maybe thats bad practice. let me know. Cheers.

brayn7’s picture

Here is something I am curious what others are doing.

Backstory, I was originally passing most things as properties to an include when @Pierre (pdureau) pointed out it would be better to use slots for most things and prevent prop drilling of sorts. I think this is a great idea and I am on the path to solving it and wrapping my head around it completely and refactoring as we go. Here is what I am coming across:

Property context may get contaminated when not using ONLY or with_context:false.
When using ONLY or with_context: TRUE I am blocked from passing a content.field_item to a embed block.

// block--component.html.twig
{% embed 'theme:my-component' with {option} %}
  {% block slot_1 %}
     {{ content.field_item.0 }} // when ONLY is used I can't do this if not then contamination happens.
  {% endblock %}
{% endembed %}

What is everyone doing in this situation?
Here is what I thought of as a workaround for the time being maybe

// block--my-component.html.twig
{% embed 'theme:my-component' with {
  option,
  _content: content // create a namespace that shouldn't collide only in files inside the template dir //(drupal templates not sdcs)
} only %} // add only back
  {% block slot_1 %}
     {{ content.field_item.0 }} // when ONLY is used I can't do this if not then contamination happens.
  {% endblock %}
{% endembed %}
nicolasgraph’s picture

@brayn7, I also do the same, or kind of, using a fake "slots" prop to store an associative array of variables needed in slots.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.