Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add trigger specification cache to Htmxor #44

Merged
merged 18 commits into from
May 13, 2024
Merged

Conversation

tanczosm
Copy link
Collaborator

This update introduces a trigger specification cache to the Htmxor library. The new feature improves parsing performance by storing evaluated trigger specifications, albeit at the cost of increased memory usage on client side (htmx's words)

I haven't added all comments or tests yet.

The changes include:

  • Addition of TriggerSpecificationCache class and related methods in HtmxConfig.cs
  • Creation of new files HtmxTriggerSpecification.cs, ITriggerBuilder.cs, TriggerBuilder.cs, TriggerModifierBuilder.cs and TriggerQueueOption.cs to handle trigger specifications

The original rationale behind adding a trigger cache to htmx was because if you have a large number of rows with a common trigger there is a sizeable performance impact when htmx has to parse each of the trigger definitions. Htmx added a key/value store to quickly look up the parsed trigger definition from the raw trigger value.

The end result looks like this within the configuration. Note that because triggers can be complex it gets a bit tricky for developers to get the cached definitions parsed correctly by hand:

{
  "useTemplateFragments": true,
  "selfRequestsOnly": true,
  "triggerSpecsCache": {
    "revealed": [
      {
        "trigger": "revealed"
      }
    ],
    "newContact from:body": [
      {
        "trigger": "newContact",
        "from": "body"
      }
    ],
    "keyup changed delay:500ms, mouseenter once": [
      {
        "trigger": "keyup",
        "changed": true,
        "delay": 500
      },
      {
        "trigger": "mouseenter",
        "once": true
      }
    ],
    "every 30s, click": [
      {
        "trigger": "every",
        "pollInterval": 30000
      },
      {
        "trigger": "click"
      }
    ]
  }
}

This is generated by this code:

builder.Services.AddRazorComponents().AddHtmx(options =>
{
	// Enabled to support out of band updates
	options.UseTemplateFragments = true;

	// Enabled to show use of trigger specification cache
	options.TriggerSpecsCache = new TriggerSpecificationCache (
		Trigger.Revealed(), // Used in InfiniteScroll demo
		Trigger.OnEvent("newContact").From("body"), // Used in TriggeringEvents demo
		Trigger.OnEvent("keyup").Changed().Delay(TimeSpan.FromMilliseconds(500))
			.Or()
			.OnEvent("mouseenter").Once(),  //  Unused, demonstrates complex trigger
		Trigger.Every(TimeSpan.FromSeconds(30)) // Unused, demonstrates use of Every
			.Or()
			.OnEvent("click")
	);
});

The trigger builder builds the complete trigger value as well as the specification used for caching. It can be added easily to the TriggerSpecificationCache as a comma-separated list of fluent trigger definitions.

Second, the builder can be used directly in code to define triggers. This is important because it makes sure the exact trigger definition is used in both the cache definition as well as in the markup:

        <HtmxFragment Match=@(req => req.Target == "contacts-table")>
            <tbody id="contacts-table" hx-get hx-trigger=@Trigger.OnEvent("newContact").From("body") hx-swap=@SwapStyles.OuterHTML>
                @foreach (var contact in Contacts.Data.Values.OrderBy(x => x.Modified).Take(20))
                {
                    <tr>
                        <td>@contact.FirstName</td>
                        <td>@contact.LastName</td>
                        <td>@contact.Email</td>
                    </tr>
                }
            </tbody>
        </HtmxFragment>

This update introduces a trigger specification cache to the Htmxor library. The new feature improves parsing performance by storing evaluated trigger specifications, albeit at the cost of increased memory usage. Users can define a simple object for a never-clearing cache or implement their own system using a proxy object.

The changes include:
- Addition of `TriggerSpecificationCache` class and related methods in `HtmxConfig.cs`
- Creation of new files `HtmxTriggerSpecification.cs`, `ITriggerBuilder.cs`, `TriggerBuilder.cs`, `TriggerModifierBuilder.cs` and `TriggerQueueOption.cs` to handle trigger specifications
- Update in the Program file to show use of this new feature with keyup and mouseenter events
Refactored the trigger specification in HtmxorExamples. Added static Trigger class to simplify creation of triggers that provides more fluent methods for constructing triggers.
Updated the hx-trigger method calls in several Razor components to use a more structured approach. This includes changes in InfiniteScroll, TriggeringEvents, and TriggeringEventsIndirectly examples. Also updated the TriggerSpecificationCache in Program.cs to reflect these changes and demonstrate usage of different triggers.
Updated the way trigger events are handled in the code. Replaced 'WithCondition' method with 'From'.
@egil
Copy link
Owner

egil commented May 10, 2024

This looks promising. A few quick notes:

  • do we need both interfaces and public types?
  • if all cached triggers can be shared on the server, not just in the client, it would save allocations on each request.

Adds documentation for each of the TriggerBuilder and TriggerModifierBuilder methods
Added comprehensive unit tests for the `TriggerBuilder` and `TriggerSpecificationCache` classes. These tests cover all methods in both classes, ensuring correct functionality of trigger creation, modification, and caching. The tests also validate the correct JSON serialization of triggers.
@tanczosm
Copy link
Collaborator Author

This looks promising. A few quick notes:

  • do we need both interfaces and public types?
  • if all cached triggers can be shared on the server, not just in the client, it would save allocations on each request.

The trigger builder consists of the base trigger builder and another class to build the modifier chains so the next fluent option is valid for the step you are on. Because you can end the trigger when in the TriggerBuilder or TriggerModifierBuilder I added ITriggerBuilder to define the build method so either could be used to add cache options.

One of the TriggerSpecificationCache constructors takes a params list of ITriggerBuilder objects.

	public TriggerSpecificationCache(params ITriggerBuilder[] builders)
	{
		builders ??= Array.Empty<ITriggerBuilder>();

		foreach (var triggerBuilder in builders)
		{
			var trigger = triggerBuilder.Build();
			Add(trigger.Key, trigger.Value);
		}
	}

That allows for:

new TriggerSpecificationCache (
		Trigger.Revealed(), // <-- this is a TriggerBuilder
		Trigger.OnEvent("newContact").From("body") // this is a TriggerModifierBuilder
	);

The base TriggerBuilder holds the whole trigger definition list so TriggerModifierBuilder's build method calls the TriggerBuilder build() method.

if all cached triggers can be shared on the server, not just in the client, it would save allocations on each request.

I'm not sure what you are asking. You can create a proxy object to perhaps download the cache list from the server. We'd have to set up an endpoint to output the cache and write some code to configure the setting. I'd put this in the future consideration list of features for now to fully implement

Added new constant values for triggers and trigger modifiers in the Constants class. Updated TriggerBuilder to use these constants instead of hardcoded strings, improving code maintainability and consistency.
Reformatted the `TriggerBuilderTests.cs` and `TriggerSpecificationCacheTests.cs` test files to improve code readability. No changes were made to the logic or functionality of the tests.
Adjusted the indentation in several files to improve readability. Also, corrected a typographical error in a comment.
@tanczosm tanczosm marked this pull request as ready for review May 12, 2024 21:43
Renamed the variable 'selector' to 'cssSelector' across multiple methods in SwapStyleBuilder and SwapStyleBuilderExtension classes for better clarity and understanding of its purpose. This change does not affect the functionality of the code.
Updated the descriptions of 'cssSelector' parameters in various methods of the SwapStyleBuilderExtension class. The term "CSS cssSelector" has been replaced with "css selector" for clarity and consistency throughout the documentation comments.
… methods

The cssSelector parameter in Scroll, ScrollTop, and ScrollBottom methods of the SwapStyleBuilderExtension class is now optional.
@tanczosm
Copy link
Collaborator Author

Let me know if you need any additional changes.

@egil
Copy link
Owner

egil commented May 13, 2024

Let me know if you need any additional changes.

Taking a look tonight and will push some changes.

Copy link
Owner

@egil egil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is good work!

A few notes on my changes:

  • I prefer not to explicitly write "// arrange // act // assert" unless the test method grows large. Simply having an empty line between each section is enough to indicate which is which. Only if there is a need for multiple empty lines I use the comments.
  • Any public type or interface added to the library has to be supported and cannot be changed after a v1 stable release, due to semver rules. The more types that can be kept internal or sealed, the better. The interface did not add any value.
  • Tests for types in the test project should be in the same folder structure as the production code they are testing. I had forgotten to move some of the test types earlier, but I did so now.
  • When writing tests, I follow the convention that the thing being tested is abbreviated "SUT", as in system-under-test. For Blazor components, the convention is "CUT" as in component-under-test.

@egil egil enabled auto-merge (rebase) May 13, 2024 23:48
@egil
Copy link
Owner

egil commented May 13, 2024

I'm not sure what you are asking. You can create a proxy object to perhaps download the cache list from the server. We'd have to set up an endpoint to output the cache and write some code to configure the setting. I'd put this in the future consideration list of features for now to fully implement

I was thinking of a micro-optimization, where we avoid allocating new builder instances when its not needed. Some of the options combinations are static. We can look at this later in the project life cycle.

@egil egil disabled auto-merge May 13, 2024 23:54
@egil egil merged commit 3710af2 into main May 13, 2024
11 checks passed
@egil egil deleted the feat-trigger-config branch May 13, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants