Twig Filter "spaceless" is deprecated

finnsky → changed the visibility of the branch 3477375-twig-filter-spaceless to hidden.

Did a pass on this, great work!

Have some questions on ~ tag, some incinsistentencies when we do the - for spaceless and would prefer less magic in the expected strings of the tests. I'm not that well versed in twig :)

I've rebased and answered some questions.

Went through the awnsers and replied. Also needs a rebase :)

@bbrala
please take a look at failed test output. can you tell me what is wrong? :)

https://issue.pages.drupalcode.org/-/drupal-3477375/-/jobs/2898571/artif...

Hmm, very weird.

What im thinking is it might be the fact that we have real vs character newlines. I tried to get the same error locally on that test, didnt workt.

We could try making it real newlines:

$expected = '<div class="link-item"><div class="link-url"><a href="' . Html::escape($url) . '">' . Html::escape($url_title) . '</a></div>\n </div>';

change to

$expected = '<div class="link-item"><div class="link-url"><a href="' . Html::escape($url) . '">' . Html::escape($url_title) . '</a></div>
 </div>';

or

$expected = '<div class="link-item"><div class="link-url"><a href="' . Html::escape($url) . '">' . Html::escape($url_title) . "</a></div>\n </div>";

And see if that resolves it.

@bbrala, probably let's create followup issue here? And for now will test whitespace/newlines trimmed results?

I've done a very small update, replacing \n with PHP_EOL and seems tests are green. This is technically fully your solution just changes the newlines.

See: https://git.drupalcode.org/project/drupal/-/merge_requests/9665/diffs?co...

Will need to check if we have adressed all comments though, i'll get backl to that, or someone else might.

We still have dropdown with same trim spaces

I will do an extra iteration, i think we could make tests work with only a very small change.

Yay got all green, now just some small optimizations to minimize the changes.

Tests are all green, and the only thing changes in the tests are a few newlines.

I can't RTBC anymore though. I'll go find someone for that :)

The MR needs a rebase.

just did it.

#20 xposted with the actual rebase. RTBC.

This is eventually going to break quite a few, if not the majority of custom and contributed Drupal themes.

Is this something that automation will be able to help with? Just a couple years ago, people were explicitly advised to use the filter, as the tag was going away.

Unless I am missing something a lot of the {{- ... -}} changes are pointless here, where the Twig tag is directly next to another tag which no intervening whitespace?

Hmm, maybe. I could check but that will take some time. Sometimes the resulsting data might also have whitespace.

I'll try and make some time to go through each and try if can be removed.

Steps:

1. Find candidate
2. Remove and push
3. Wait for tests
4. Next

Unfortunately the test doesn't want to run locally for me. :(

I intially throught it would also remove whitespace between html tags in the variable. Let's see how wrong i was :)

This is eventually going to break quite a few, if not the majority of custom and contributed Drupal themes.

Is this something that automation will be able to help with? Just a couple years ago, people were explicitly advised to use the filter, as the tag was going away.

Hmmz, yeah it is gonna be quite discurptive. Basically we need to remove the spaceless tags, which would be fine. Its easy enough to make a script that removes those tags in twig templates. So in a way this could be added to update bot to fix. Although it would be something "special" since it wouldn't be rector really. So what the correct place for that is, that is something that needs some thought.

All green without most of the whitespace stuff. @longwave did raise the question if these changes could lead to layout shifts. I find that hard to know. Hopefully someone can say something smart about that.

bbrala → credited casey → .

bbrala → credited timohuisman → .

Adding credit for 2 colleagues who went through it all.

I've fixed all the issues timo and cees identified. Think every single change has now been reasoned about. Ready for review again.

Added some credits. Commiter needs to decide on @mondrake

All the raised concerns that we discussed this morning are resolved. RTBC

Please do not credit me. The review I did was surfacing and not the deep dive that was required.

This adds unwanted whitespace to the author byline on nodes in Olivero:

Before the MR:

After the MR:

I did not manually check all other uses; I wonder if we need to break this down a bit and check all cases individually :(

Arg that's pretty annoying. We were hoping the fact is uses link template would remove all spaces around that.

Only one behind it it seems?

I also got nod to promise to take a look, he'll know what to expect.

I'll do an install and fix the author. See if I can validate a few, just worried I cannot replicate all templates. Hopefully have time for that tonight.

ANother one

<div class="tabledrag-cell-content js-tabledrag-cell-content"><a href="#" title="Change order" class="tabledrag-handle js-tabledrag-handle tabledrag-handle-y"></a><div class="tabledrag-cell-content__item">Primary admin actions</div></div>

Height is wrong for the rows.

Draggable rows seem misaligned, more than that i cannot find.

Seems like we'll have to backport spaceless to our repo:

1. Normal HTML behavior: a new line is rendered as a space, which cause the problem with "submitted by"
2. Extra Drupal spice on the issue: when twig debug is turned on, it adds newlines all over the place

I do not see a way to make this work with only twig whitespace control features.

@nod_ do you think it's still worth trying to discourage it somehow and remove it from e.g. admin type templates where the whitespace doesn't actually matter?

it's a good idea to remove it where it's not needed. I don't think it's worth it to remove it at all costs though, it's applied on pretty small strings, not whole pages so performance impact shouldn't be too bad to begin with. We're not using this to minify a whole page so that doesn't appy to us.

If we need to keep it, I wonder if there is a way our then by overriding the spaceless tag so we don't trigger the deprecation. This would also be very helpfull for theres who might use it.

We might be able to something like we do in 📌 Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.12.0 Active then and override the spaceless filter.

This is how it is called in Twig itself:

new TwigFilter('spaceless', [self::class, 'spaceless'], ['is_safe' => ['html'], 'deprecated' => '3.12', 'deprecating_package' => 'twig/twig']),

Made a quick MR to test replacing the filter.

Ok, that approach will need more work. Was hoping for a quick one. Don't want to dive into my debugger right now and see when we can inject or override somehow.

Personally i would also rather use it only when needed like longwave suggests. There are quite a few places where it could be quite harmless to remove.

Well, it does work, but since the old one is loaded anyways before we replace it in the visitor it does actually trigger the deprecation. So not sure if we can even fully disable the deprecated one in CoreExtentions and have no error.

This might mean we need to actually replace the tag.

May be a way out if we would load the druapl extentions before twig core extention, but that would mean maintaining the full contructor.

Hmmz. Can't we fix the debug output to not add newlines before/after or something? Or is there way more places where this will become an issue?

if we remove the newlines from debug output it'll be unreadable and will defeat the purpose of having it in the first place. It needs to be human readable output with how things are today. If we had a dedicated debugger or inspector we could get around that but we're not there yet and I don't know of anyone working on such tooling.

Isnt the issue that there is extra newlines before/after/between tags, not nessesarily in the comments. The comment could still be formatted, we just need the output not to use newlines around them? We could even apply a drupal_spaceless filter on that specific node when it is visited by our nodevisitor and have that output be ok? But then again, we might as well replace apply spaceless with apply drupal_spaceless and start helping contrib move along.

Still though, that would not solve the fact it is deprecated and the fun contrib will have with spaceless being gone. Been looking at replacing it with out own, but the deprecation error is thrown when when the parser is doing its work. Since we cannot without to much effort get into that part of twig seems we cannot really do much.

Is there potential here to approach this a bit more seamlessly?

For example, can we detect if spaceless exists upstream and only if it does not, add a twig function that matches the contract 100%?

  if (upstream spaceless does not exist) {
    // Replace removed upstream spaceless filter with our own.
    new TwigFilter('spaceless', [self::class, 'spacelessFilter'], ['is_safe' => ['html']]),
  }

This would mean zero disruption to downstream clients.

If we want to keep spaceless, what about @lleber's conditionally defined filter as well as setting a custom error handler before we call Twig to suppress the deprecation message?

The solution with the drupal_spaceless would work, the issue is that the deprecation message is triggered in the ExpressionParser. This is really early in the process, and i have not found a way to really hook into that early enough.

We might be able to do some 'hackery' to adjust the specific filter in CoreExtention to not have deprecated. But i've not found a way for that.

@luke.leber

your #55 assumption is incorrect

{% set var = '<span>Hello</span> <span>There!</span>' %}
{{- var|raw -}} -> renders Hello There!

Spaces trimmed from begin and end

I was also thinking of using composer autoload to override a class to fix this, but that would mean overwriting a big class like CoreExtension, or smaller TwigFilter, but that does kinda feal like using a hammer to screw in a screw. We could though override TwigFilter, its quite a small class, but that adds complexity when updating at some point.

Re- #58

See https://fiddle.nette.org/twig/#3c6dbf089c

There is certainly changes in the generated output such that whitespace control operators are not a direct replacement for spaceless.

catch makes a good point in #56. The if statement could probably just be a version check on the twig dependency.

My main point here in #55 is that we probably shouldn't reimplement spaceless with a new filter name, as that would just cause needless untold fatal errors in custom and contributes themes on upgrade where it could otherwise be avoided if the same filter name is used.

re #60 the fiddle is using outdated version (3.7.1) but deprecation is since v3.12.0 and using PHP backend (not JS)

I've tested templates before/after patch.
WIth/without twig debug.
Templates without regressions marked with +

core/modules/block_content/templates/block-content-add-list.html.twig +
core/modules/ckeditor5/templates/ckeditor5-settings-toolbar.html.twig +

core/modules/link/templates/link-formatter-link-separate.html.twig
fixed(looks as before now)

core/modules/link/tests/themes/link_test_theme/templates/link-formatter-link-separate.html.twig
revered `-` signs there. Now works as before with/without twig debug

core/modules/system/templates/dropbutton-wrapper.html.twig +
core/modules/system/templates/select.html.twig +
core/modules/toolbar/templates/toolbar.html.twig +
core/profiles/demo_umami/themes/umami/templates/classy/field/link-formatter-link-separate.html.twig +
core/profiles/demo_umami/themes/umami/templates/classy/navigation/toolbar.html.twig +
core/themes/claro/templates/classy/field/link-formatter-link-separate.html.twig +
core/themes/claro/templates/classy/navigation/toolbar.html.twig +
core/themes/claro/templates/form/input.html.twig +
core/themes/claro/templates/navigation/toolbar.html.twig +
core/themes/claro/templates/views/views-mini-pager.html.twig +
core/themes/claro/templates/pager.html.twig +

core/themes/olivero/templates/content/node--teaser.html.twig
fixed visual regression with css
it related to core/modules/node/templates/field--node--uid.html.twig which i don't want to change here

core/themes/olivero/templates/content/node.html.twig
same CSS fix plus extra wrapper.

core/themes/olivero/templates/content/search-result.html.twig +
core/themes/olivero/templates/navigation/pager.html.twig +
core/themes/olivero/templates/user/username.html.twig +
core/themes/olivero/templates/views/views-mini-pager.html.twig +
core/themes/stable9/templates/admin/block-content-add-list.html.twig +

core/themes/stable9/templates/admin/block-content-add-list.html.twig +

core/themes/stable9/templates/field/link-formatter-link-separate.html.twig
fixed(looks as before now)

core/themes/stable9/templates/form/dropbutton-wrapper.html.twig +
core/themes/stable9/templates/form/select.html.twig +
core/themes/stable9/templates/navigation/toolbar.html.twig +

starterkit didn't checked. just reapplied fixes

Probably this custom filter can be moved to temporary module?
To make contributors life easier.
Core obviously doesn't need it. All regressions fixed

Does it work when debug comments are present?

yes, i've tested in both modes

Revisiting the idea of replacing the spaceless filter: It happens that if a Twig extension is added to the environment that has a filter with the same name as a filter in an extension already added to the environment, then the filter definition in the extension added last takes precedence. I've added a commit to MR 9735 that adds a SpacelessBCExtension class after Twig's CoreExtension is added to the Twig Environment. The spaceless filter is currently defined in SpacelessBCExtension without deprecation warnings, and all tests now pass. Could also change it to issue a different deprecation message to use drupal_spaceless instead by Drupal 12.

I'm so confused. I tried that earlier. But yay!

Should the effort to replace all usage of spaceless in core templates (MR 9665) and the effort to suppress the spaceless filter deprecation warning + add drupal_spaceless filter (MR 9735) be combined into one MR?

Also, should Drupal have its own deprecation message for spaceless usage, with a recommendation to change to drupal_spaceless?

What is the benefit of changing the name of the spaceless filter in a non-bc way?

Maybe I'm dense, but not causing excess contrib and custom disruption should be a chief consideration in the decision making process, right?

The `spaceless` filter in Twig was deprecated as of version 3.12. To address this deprecation, you should remove the `spaceless` filter from your Twig templates. Instead, rely on HTML minification tools, like AdvAgg or HTML Minifier, or front-end build tools to minimize whitespace in your output. Additionally, CSS can be used to manage whitespace rendering. Removing the filter ensures compatibility with future Twig versions.

Hello,

Verified on Drupal 11 without patch its looks fine for me.
Not able to reproduce getting error.
Attaching screenshots of without patch and error.

Yeah we had this issue in the footnotes module ( 🐛 Fix deprecated spaceless issue Active ) because spaceless is quite important to avoid inline citations having spaces around them. Perfectly happy though to have a mini footnotes_spaceless twig filter to cover that. If it turns out its needed by more modules, we can always switch to depending on something in contrib.

issue summary is not up to date. We should probably open a different issue to remove the use of spaceless in core and leave this one to deal with the deprecation or polyfill for contrib is needed.

Can someone open an issue with the code from MR !9665 ? thanks!

godotislate → changed the visibility of the branch 3477375-spaceless to hidden.

Updated IS, created follow up to remove spaceless usage in core ( 📌 Remove use of deprecated "spaceless" filter in core templates Active ), and added a test that the spaceless filter still works as expected.

Hello,

Verified on Drupal 11 without patch its looks fine for me.
Attaching screenshots.

RTBC

Moving to RTBC per #77 📌 Twig Filter "spaceless" is deprecated Active .

This looks great everyone, nice work!

rebased to bring it up to date with the latest 11.x

Is this needed now that 📌 Remove use of deprecated "spaceless" filter in core templates Active has been committed? spaceless no longer appears in Drupal core.

The deprecation/removal in the future will cause another major rework of twig templates in contrib/custom modules and themes. I think the goal was that core can keep it for BC reason and it to avoid this disruption.

@liam morland: 📌 Remove use of deprecated "spaceless" filter in core templates Active was split from this issue per request in #74 📌 Twig Filter "spaceless" is deprecated Active .