DX object to manage htmx attributes

All tests green

Missed bringing over the Twig function

New test passes.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Applied a suggestion and will refactoring some improvements inspired by @larowan review. A maintainer should add him to the credits.

All tests green.

There are two open questions:

1. The best way to ensure a string is returned in AttributeJson
2. Single/double quoted syntax. See also filtering setup for strings in AttributeHtmxString.

I don't understand why we need the single quote. If you add a JSON::encode(), to the process assets MR it just works.

    $build['replace'] = [
      '#type' => 'html_tag',
      '#tag' => 'button',
      '#attributes' => [
        'type' => 'button',
        'name' => 'replace',
        'data-hx-get' => $url->toString(),
        'data-hx-select' => 'div.ajax-content',
        'data-hx-target' => '[data-drupal-htmx-target]',
// add this part
        'data-hx-vals' => Json::encode(['test' => 'ok']),
      ],
      '#value' => 'Click this',
      '#attached' => [
        'library' => [
          'core/drupal.htmx',
        ],
      ],
    ];

It doesn't matter that the source html doesn't look pretty. Once it's in the JS it's the correct value.

That's a great question! I didn't think it would but I didn't try. I'll repeat the experiment with an attribute value selector and remove the complexity. Simpler is better!!

Confirmed! I can now remove a trait and a class - thank you @nod_ :)

I altered the render arrays in the test module to

    $build['replace'] = [
      '#type' => 'html_tag',
      '#tag' => 'button',
      '#attributes' => [
        'type' => 'button',
        'name' => 'replace',
        'data-hx-get' => $url->toString(),
        'data-hx-select' => 'div.ajax-content',
        'data-hx-target' => 'div[data-drupal-htmx-target="insert-here"]',
      ],
      '#value' => 'Click this',
      '#attached' => [
        'library' => [
          'core/drupal.htmx',
        ],
      ],
    ];

    $build['content'] = [
      '#type' => 'container',
      '#attributes' => [
        'data-drupal-htmx-target' => 'insert-here',
        'class' => ['htmx-test-container'],
      ],

Which produces data-hx-target="div[data-drupal-htmx-target="insert-here"]" through the standard string attribute. HTMX worked as expected.

All threads resolved - all tests passing :)

We agreed in Slack to pause on this builder while we work on 📌 Refactor BigPipe to use HTMX Active using attributes that are "hand-rolled."

Coming back to this now - we needed the htmx javascript API in 📌 Refactor BigPipe to use HTMX Active but not attributes.

Tagged \Drupal\Core\Template\HtmlAttributeInterface as @internal as that seems consistent with the policy on Attribute class internals → .

Discussed and agreed in Slack that a single change record and release note will cover:

• 📌 DX object to manage htmx attributes Active
• 📌 DX object to manage htmx headers Active
• 📌 DX object to collect and manage HTMX behaviors Active
• 📌 Define and process an #htmx render array key Active

Based on #26 and the description of the tag (A change record needs to be drafted before an issue is committed.) I'm removing the CR tag from this issue as these are sequential dependencies and the tag belongs on final component.

Reading through the governance doc I am marking this for framework manager review since it is blocking other work.

I brought the summary up to date with the code. We have 5 unresolved comment threads on the MR. Most open threads have a response.

Following a comment from @catch I updated the code in \Drupal\Core\Template\HtmxAttribute for:

• ::get
• ::post
• ::put
• ::patch
• ::delete
• ::pushUrl
• ::replaceUrl

to pass in and instance of CacheableDependencyInterface and collect the cacheable metadata emitted by the Url object.

There is an open question about the advisability of adding return types in \Drupal\Core\Template\Attribute. I understand this class to be internal by policy but maybe that's not right?

The other 3 open threads all have answers.

I have time to move this project forward to let's get this to RTBC so it can be committed and work can start on the next layer!!

@catch can you give guidance about adding return types to \Drupal\Core\Template\Attribute?

I hit a name collision on the BC policy with the Attribute name (As you and @nicxvan noted that was for PHP attributes). But it also seems to be permitted with these:

PHP and JavaScript classes →
In general, only interfaces can be relied on as the public API. With the exception of base classes, developers should generally assume that implementations of classes will change, and that they should not inherit from classes in the system, or be prepared to update code for changes if they do. See also more specific notes below.

Public methods not specified in an interface →
Public methods on a class that aren't on a corresponding interface are considered @internal.

We are adding an interface in this issue, but there no interface for this class now. @larowan seemed to think it was okay here: https://git.drupalcode.org/project/drupal/-/merge_requests/12097#note_51...

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Thank you @phenaproxima for improving the code with your questions and suggestions. I've resolved all comments with either updates or answers except those related to return types (see #30). All tests are passing.

Given @larowlan's:

I did a search of contrib for classes that extend this where these new return types might be a BC break and could only find on in display suite, that was only adding a new method - and one in htmx which also didn't override any of the methods we're adding too here So I think this change is fine to do in a minor release, with a change record

and https://www.drupal.org/about/core/policies/core-change-policies/bc-polic... → I think we're OK to add the return type hints to Attribute here with a change record. It's allowed under the bc policy, and we can be fairly confident it won't break any existing code.

Ok that was tedious, but I clicked on all documentation links and confirmed they align except the ones that I have suggestions for.

I also reviewed all changes and have been following. I think this is ready assuming the three suggestions are accepted and tests pass.

Only remaining substantial question I have is where does sanitization happen?
I see some attributes do check the expected value is a boolean, but I don't see anything else, is that all handled upstream of this?

@nicxvan First, thank you for so carefully checking the tedious mundane details! Suggestions committed and test updated for the one method name fix.

Sanitization happens in the attribute value object's ::__toString method. They all extend AttributeValueBase which sanitizes the name property. We add AttributeJson in this MR, and follow this pattern.

I think this is ready, the extensive feedback has been addressed!

@catch @larowlan @nod_ A CacheableMetada parameter was added to methods that take a URL based on feedback. This complicates calling these methods within Twig as the caching has already bubbled.

Does this change the utility of a providing the attribute object within Twig? Should we drop that function from this MR?

I'd drop it. We can always add a followup to add it and we can figure out the problems without holding the working part of the MR

Thanks @nod_ !!

Removed the twig function from this issue's MR and the change record. It is extra.

CR is updated with the full picture of how all 4 issue changes work together.

few cosmetic feedbacks

few cosmetic feedbacks

Suggested changes made.

Unrelated toolbar tests failing. Rebasing onto HEAD.

Required tests passing. The php 8.5 test seems to be failing in general due to widespread deprecations.

I think this is rtbc, I reviewed the comments from nod and the two commits since my initial rtbc.

We need the empty URL thing for the post/patch/delete/put methods too

Re: #48 - okay. It's not documented at

• https://htmx.org/attributes/hx-post/
• https://htmx.org/attributes/hx-delete/
• https://htmx.org/attributes/hx-patch/
• https://htmx.org/attributes/hx-put/

as it is for get. Did you find support for all five in the code @nod_? It will make our code simpler so I'm going ahead. It would be good to document in a comment here for future reference with a link to their code or other documentation.

Made all five http verb methods operate the same way.

All tests passing

The empty URL behavior discussed in #48 & #49 is in the issueAjaxRequest function of htmx.js As of this note this can be seen at https://github.com/bigskysoftware/htmx/blob/master/src/htmx.js#L4426

Turns out this goes back to RFC 2396 on the URI specification co-authored by Tim Berners-Lee in 1998 no less!

4.2. Same-document References
A URI reference that does not contain a URI is a reference to the current document...

We've had a discussion on slack with @fathershawn about this.

The current approach of the MR is not ideal:

• All the HTMX-specific helpers and documentation lives in the \Drupal\Core\Template namespace
• Values are transformed to an AttributeValueBase right away, making it unusual to work with in alter/preprocesses

Thankfully we can fix it by moving things around:

• Move the builder object to the \Drupal\Core\Htmx namespace
• Make it work more like CacheableMetadata, with an applyTo() and createFromRenderArray (we need this to unserialize the json attributes) methods that we call on a render array to augment it.
• In the applyTo method, save all the data as attributes in the render array #attributes key, and json_encode what needs to be json_encoded and let the existing code handle the attribute rendering

Values are transformed to an AttributeValueBase right away, making it unusual to work with in alter/preprocesses

Great catch!
Is there a test we can add showing this, or does it need implementation for that?

fathershawn → closed merge request !12097

nod_ → credited larowlan → .

nod_ → credited phenaproxima → .

@nicxvan not sure how we'd test this reliably. Maybe check if there are any AttributeValueBase in the #attributes array at the preprocess step? but then which render array do we test? or maybe a bunch of asserts?

@fathershawn: now that you're a maintainer you can give credit, credits are given for closed issues now too :)

In the applyTo method, save all the data as attributes in the render array #attributes key, and json_encode what needs to be json_encoded and let the existing code handle the attribute rendering

I am not familiar about the applyTo method from CacheableMetadata, is it resolved late? Ideally, we keep the HTML values separated as long as possible and we move them to #attributes just before rendering.

We are working on at least on two other features with attributes values which must be set and kept distinct from #attributes but merged to #attributesbefore rendering: ✨ Add a style utility API Active and ✨ Add a CSS variables API Active . So a generic behavior and/or standard "way" may be useful.