egil / Htmxor

Supercharges Blazor static server side rendering (SSR) by seamlessly integrating the Htmx.org frontend library.
MIT License
124 stars 13 forks source link

refactor: Migrate SwapStyles enum to SwapStyles strongly typed object #49

Closed tanczosm closed 4 months ago

tanczosm commented 4 months ago

Problem

Before hitting 1.0 final one of the odd things that is going to pop up is using the swap style builder as a helper for hx-swap attributes. At the moment it will work for hx-swap attributes that utilize the builder but if you just use the enum it will provide incorrect string conversions.

<div hx-get="/home" hx-swap=@SwapStyle.OuterHTML>  -- doesn't work because enum with conversion is "OuterHTML"
<div hx-get="/home" hx-swap=@SwapStyle.OuterHTML.ScrollFocus()>  -- does work because the builder returns proper strings

So the alternative is to use the defined constants as you did a few times:

<div hx-get="/home" hx-swap=@SwapStyles.OuterHTML> // or
<div hx-get="/home" hx-swap=@Constants.SwapStyles.OuterHTML>

But then if you later try to extend the swapstyle using the builder you won't get any intellisense because the builder uses SwapStyle rather than SwapStyles.

Proposed Solution

I created a strongly-typed object called SwapStyle to replace the SwapStyle enum that creates an object with an underlying string and numeric value that works identically to the enum. The downside is that as a complex object it's not a compile time constant that can be used in method signatures, but the only defaults that impacted were SwapStyle.Default, which itself an empty string. I changed those parameters to be nullable and used SwapStyle.Default if the parameter was null.

Pros

Cons

For the cons side, this can be used as a workaround if used as a method parameter:

public void SomeMethod (string style = Constant.SwapStyles.InnerHTML) {}

SomeMethod(SwapStyle.InnerHTML)

This all works:

var style = SwapStyle.OuterHTML;
style = SwapStyle.BeforeBegin;
style = Constants.SwapStyles.Delete;  // implicit string conversion of a string style to the typed value

if (style == SwapStyle.Delete) // true
{
}

if (style == "delete") // also true
{

}
egil commented 4 months ago

Yeah, it's a bit of a mess right now with too many ways to do the same thing.

I do like the builder, especially for complex swaps with modifiers, but I also want to be performance-conscious, and the builder is quite expensive compared to just using enum or constants.

The enum option has the advantage of not being "stringly-typed", but will require a small modification to the renderer such that if it is used directly in an attribute, it produces the correct markup.

E.g.: hx-swap=@SwapStyle.BeforeBegin renders as hx-swap="beforebegin".

That is a simple addition I can make to the renderer.

The SwapStyle enum can also be used as a starting point for the builder, which it already is, such that users can start with the enum and "pay the cost" of using the builder if they need the additional help.

I am in favor of changing the constants we that overlap with enums to internal, to make the API simpler for the end user.

tanczosm commented 4 months ago

I like that idea better if you can mark constants internal and alter the renderer.

egil commented 4 months ago

I'll push a commit later that does this.