I still need to deal with Messages and Redirects, but this approach is simpler and over a 10 request sample results in a mean time of Load of 178.6 ms compared to 185.4 ms for the existing 11.x branch.
Here's an MR into @nod_'s branch moving the bigPipe command JS into bigPipe scope. It looks like the test setup scripts don't like MR's into a feature branch. The tests generally pass locally - 2 failures involving noJS exceptions. Not sure if that's a false negative as all of these changes are in JS.
If this is an acceptable direction then I think the next level of refactoring is to create the needed bigPipe Command classes to replace the AjaxCommands and update BigPipe.php to work without the Ajax API classes.
fathershawn → changed the visibility of the branch 3526267-bigpipe-with-htmx to hidden.
We need this only if if there is an existing capability we cannot implement using the tools provided by htmx - let's keep it a placeholder.
All green is super - nice work! This is direction really simplifies the changes in big_pipe.js and that’s a real plus.
Because bigPipe as we have it re-uses existing javascript from the ajax system, to remove the ajax system we need to replace that.
@nod_ had this list in a Slack discussion:
We need the equivalent of those 3 commands to implement bigpipe
- use Drupal\Core\Ajax\MessageCommand;
- use Drupal\Core\Ajax\RedirectCommand;
- use Drupal\Core\Ajax\ReplaceCommand;
- Add_css/add_js and the settings but that’s mostly available already
I think we need to identify the pieces of this list that htmx is not going to do, and implement those in general scope. Then the items that htmx does just fine in an ajax context, we need to have as helper methods in big_pipe.js and leverage the htmx api where we can.
I'm going to take them in order.
MessageCommand
In the ajax context, I would have implimented this in HTMX using a header and a custom event handler that calls our Drupal.Message
function. This is actually the example in the trigger headers documentation.
HX-Trigger: {"showMessage":{"level" : "info", "message" : "Here Is A Message"}}
One thing that Drupal.Message
does client-side is determine the selector for the message container. If there is a way to do that server-side then this could also be implemented as an extra out-of-band swap on the response without any additional javascript.
We do need a helper method for this that we can use in big_pipe though.
RedirectCommand
HTMX has both the HX-Redirect and HX-Location response headers. We don't have a response so we need a function for that in bigPipe.
ReplaceCommand
We also just need the bigPipe use case for bigPipe. This is one of the core competencies of htmx in an ajax request context.
addCSS and addJS
In the ajax context using htmx, these feel like out of band swaps as well.
We agreed in Slack to pause on this builder while we work on 📌 Refactor BigPipe to use HTMX Active using headers that are "hand-rolled."
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."
fathershawn → created an issue.
What ever we implement, if there is configuration we should collect in via \Drupal\dynamic_yield\Form\FeedSettingsForm
and store it in the general settings.
We already to not load DY on admin routes. The presenting issue is that some pages often use the admin theme, such as entity edit forms. This seemed like a simple way to exclude certain path patterns.
We could programmatically detect the active theme, get then name of the admin theme and compare to exclude DY.
I think all questions have been addressed.
Thank you @catch. I wanted to layout the concerns expressed to me, and I'm comfortable with our original course. I think it is cleaner to create and transition to a parallel system. The HtmxInterface
that I propose in the issue summary offers two paths for a developer.
A developer does not need to know anything about how HTMX works. Choosing from a defined set of operations, such as Replace or Insert, the developer adds the required data to the operation and inserts it into the Htmx object.
Alternatively, a developer can dive completely into the world of HTMX using the attribute and header subsystem objects that are part of the Htmx object. I guess there is also a third path which is to create their own implementation of HtmxOperationInterface
to package these into something reusable.
This offers both simplicity and power in something that I think is straightforward to manage. Creating all the operations will be our final task in the initiative. I'm confident that we can offer a migration guide that maps ajax commands to htmx operations.
Tests revealed some issues
That's the right idea :). When I have time I'll inspect the render array there.
And in fact I did that in \Drupal\htmx\Render\HtmxBlockView::build
. So now to figure out why is didn't do that for the caching
fathershawn → created an issue.
It sounds to me like what is needed is that the dedicated render controller for the HTMX block needs to use the context of the requesting page.
This is ready - let's get it in!
Looks like there are over 60 projects on d.o using this approach now.
This module uses the https://github.com/ddev/ddev-drupal-contrib approach for module development. This makes the module easy to maintain. Drupal CMS also has a committed `.ddev` directory although their setup isn't for a contrib module: https://git.drupalcode.org/project/drupal_cms
Fixed in ✨ Single-Directory Components Active
All threads resolved - all tests passing :)
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.
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!!
All tests green.
There are two open questions:
Applied a suggestion and will refactoring some improvements inspired by @larowan review. A maintainer should add him to the credits.
I had a conversation in Slack with @cilefen about how much we should present an abstract interface to the rest of the system and keep the HTMX implementation interface encapsulated. This alternate architecture would argue that we should name the facade class Ajax
as that is the concept we are implementing. We would not provide direct access to the HtmxAttribute
and HtmxHeader
subsystems. The means of altering these subsystems would be to use an operation.
Such an architecture only exposes HTMX concepts to developers that need to create a combination of attributes and headers that is not available as an operation. In that case, the developer would create a new operation to encapsulate that combination. Of course such a developer could forsake the facade altogether and deal with the attribute and header subsystem directly.
This idea could be take further in
📌
Define and process an #htmx render array key
Active
and rather than define a new render array key, only add a new callback. The existing ajax callback would make an early return if the #ajax
key did not point to an array and similarly the new callback if it did not point to an instance of the facade.
What is the right architecture?
fathershawn → created an issue.
Nice use of once
property to verify the behavior had done its work +1!
With 10.2-alpha1 out should we move the tag to 10.3 and release all the functionality together?
fathershawn → created an issue.
New test passes.
Missed bringing over the Twig function
All tests green
All tests green
It might not be, but if we refactor BigPipe to use HTMX we should return HTML - probably with the placeholder as the main content, and I wondered if that might intersect with this use case??
fathershawn → created an issue.
fathershawn → created an issue.
I'm so glad to be working with collaborators who have a historical perspective on the code! This is an enhancement not a requirement that will give a bit of performance boost. I'm going to postpone it and move it down the task list before BigPipe.
In the module, this simplified HTML response is a route option and scanning the linked history that may be a good thing. Let's think about routes we might build to serve requests from HTMX and pick up this work in a while. Ideas that I have for routes are:
- the one I have in the module
/htmx/{entityType}/{entity}/{viewMode}
- one to render placeholders as the main content
RE: #16 - yes we have that covered in 📌 Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries Active
All other unresolved MR comments are from the previous MR which we have closed.
I can't mark the MR comment resolved but I removed the version requirement from the info.yml file in the test module.
@longwave Okay, that's two experienced maintainers recommending a renderer over page variants. Thanks for weighing in!
We don't need the query string as HTMX adds the header HX-Request: true
to all requests. I'll start to work on that approach as an alternative. I think we do still want title though so that the HTMX feature of swapping the title is available if appropriate but we turn it off by default.
Rendering the system messages in the response would mean that we wouldn't need a command to define the messages. The core systems of defining and displaying messages is available.
We would need to implement in a later issue either
- Always select and display system messages as additional inserted content
- Provide an operation for selecting the messages from the response and displaying them to be used with appropriate
All tests green, including new unit test for this subscriber
Nice simple solution @nod_. All tests green including the the added Ajax interaction test. What else do we need to move to RTBC?
All tests passed
PHP 8.4 test failed
Another advantage of the page variants is that SimplePageVariant
contains system messages as well as main content. That would allow the calling element to use hx-select-oob, or for us to add an hx-swap-oob to the response, and display error messages as well as the selected content from the response.
Maybe. That doesn't necessarily look simpler to me but I don't know the internals as well as @larowlan. The work in
📌
Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries
Active
is based on returning an HtmlResponse
object, so would we be creating a simpler version of \Drupal\Core\Render\MainContent\HtmlRenderer
? I would think we would still need an event subscriber to divert the request to the alternate renderer.
Opened an MR that uses a default simple page with a header control for using a block page variant.
fathershawn → changed the visibility of the branch linting-in-progress to hidden.
fathershawn → changed the visibility of the branch 11.x to hidden.
fathershawn → created an issue.
@nod_ requested two additional test cases in the prior PR.
- htmx powered elements inserted by our legacy ajax
- verify that our attach and detach behavior methods fire
I've added a route to the test_htmx
module (/htmx-test-attachments/ajax) that creates markup for this test. In manual testing, htmx is inserted but does not process the inserted content. I think we need some glue code that calls htmx.process on markup inserted by our Ajax API.
The only way I see to verify that methods are called in the Nightwatch test is to use the callTracker assertion but this is deprecated, so I'm wondering if someone has a better approach?
I also added some documentation links to the JS files.
fathershawn → changed the visibility of the branch without-htmx-attachement-processor to hidden.
fathershawn → changed the visibility of the branch 3521173-process-attachments-cssjs to hidden.
fathershawn → changed the visibility of the branch asset-tests to hidden.
Thinking about the feedback on an additional processor service I realized that we should be able to simplify that whole part of the solution by adding some appropriate conditions to the existing HtmlResponseAttachmentsProcessor
. I'm hitting a wrinkle though which is that HtmlResponseAttachmentsProcessor
gets called more than once preparing the response to my test page. This is an issue because if I call setAlreadyLoadedLibraries
on the assets, then they stop getting added in subsequent runs.
Someone with more permissions must be needed to edit the JS Dependencies → page. I don't know if someone edited source or there are other text filters available, but I can't add additional headers and rows to the existing table.
Here's the needed data:
Repository https://github.com/bigskysoftware/htmx
Release cycle Releases are expected quarterly.
Security policies https://github.com/bigskysoftware/htmx/security
Security issue reporting https://github.com/bigskysoftware/htmx/security/advisories/new
Contact(s)
1cg →
,
fathershawn →
I hav two questions about merging drupalSettings.
ajax.js uses jQuery to merge the incoming drupalSettings value using the deep variation of .extend()
if (response.merge) {
$.extend(true, drupalSettings, response.settings);
} else {
ajax.settings = response.settings;
}
In my contrib implementation I didn't want to start with a reliance on jQuery so adapted code from https://youmightnotneedjquery.com. I've kept this pattern in the MR at the moment, but I see that jQuery.extend() is used nearly a dozen other scripts. What's the best way forward? I see
#3179551: Provide no-library equivalents of common/useful jQuery functions →
but it doesn't include .extend()
.
2. The ajax API provides two options for drupalSettings above. I currently simply merge. What is the use case for not merging incoming settings?
I've added a Nightwatch test, and all Nightwatch tests are passing.