hx-error-swap and hx-error-target attributes #1619

[Telroshan]

This PR addresses #1619, see issue for the context

here are the highlights of the suggested changes:

• 2 new attributes: hx-error-target and hx-error-swap attributes
  They both use the same syntax as, respectively, hx-target and hx-swap
• They both support an additional value, that I named mirror (feel free to suggest names, it's already hard enough to name things in my mother tongue, send help), which indicates htmx to swap errors (that would otherwise be ignored by default) using the same specifications as their standard attribute counterpart
  • Thus, hx-error-target="mirror" would use the same target as defined in hx-target, or, if no hx-target was specified in the hierarchy, fallback to the default target (being the element itself)
  • hx-error-swap="mirror" would use the same swap spec as defined in hx-target, or, if no hx-swap was specified in the hierarchy, fallback to config.defaultSwapStyle.
    • Note here that it defaults to defaultSwapStyle and not the new config parameter defaultErrorSwapStyle, for which you would just have to set no value at all, or an invalid one
• New config param: htmx.config.defaultErrorSwapStyle, that defaults to "none". This default value allows htmx to keep the behaviour it had, thus avoiding breaking changes, if we were to add that feature to htmx1
• New config param: htmx.config.defaultErrorTarget, that defaults to "mirror". This default value allows "lazy errors handling" so that by default, the errors would be swapped at the same target you had specified with the standard hx-target (given that you have defined an error swapping strategy other than "none" ofc, whether it's globally or on the element triggering the request)
• This way, if you were to set both those 2 new config parameters to "mirror", you would get content swapped like you defined using hx-target and hx-swap everywhere in your code, no matter if the request was successful, and resulted in an error
• Added a bunch of tests, most of them are hx-target and hx-swap's tests adapted to their error counterpart. Also added tests for these special mirror values. All tests pass, though let me know if you think of any edge case that isn't included there

[Renerick]

Just some thoughts out loud. Great PR @Telroshan, helps a lot!

[Renerick]

I'm a bit suspicious about passing error flag to swap method. Ideally, the decision to use the default value should come from the use site (which would be the doSwap method), but I suppose this calls for larger refactoring

[Telroshan]

Yeah that's because the swap function gets htmx.config.defaultSwapStyle in the fallback case, and I needed to differentiate from the error counterpart when relevant here
Couldn't find an elegant solution to work around this issue, other than passing that boolean around, which isn't that elegant either I know 😆

[Telroshan]

@Renerick see my latest commit, I prefer that new version, though it still requires passing a parameter down the way, but now it's the default swap style instead of a boolean, let me know what you think about that

[Renerick]

Controversial idea - allow functions as configuration values. Function is evaluated at runtime and the returned value is used as corresponding configuration value. Removes the need for these reserved magic values, but maybe a bit of an overkill for what is essentially a single use-case?

[Telroshan]

I mean, why not adding function on top of that, but I'm not convinced by them replacing the magic value.

My goal with those values was to provide an easy, quick & lazy way for the user to setup a global errors swapping strategy, so that it's easy to toggle them. Just set double mirror and you'll have errors everywhere, or let defaultErrorTarget to mirror then you'll only need to specify hx-error-swap and the target will follow hx-target everywhere, that kind of thing.

To reproduce the mirror logic with a function and without that magic value, you'd have to code yourself the retrieval of standard swap/target strategy (if applicable) in your project, so that doesn't seem to remove the need in my opinion

Also, wouldn't the preferred way for that kind of thing, be to maybe refactor the htmx:beforeSwap event and add more parameters to it, so that the dev can override the error swap parameters if they want for specific requests, using that event instead of a function global config?

Config is also supposed to be preferably set through meta tags, though maybe is that not relevant for specific use cases where you'd want your users to write some custom JS to set up their own thing anyway?

[Renerick]

Very valid points, I especially forgot about meta tags, don't think there is a good way to mix them with functions.

[Telroshan]

Added the idea we talked about yesterday @alexpetros (whitelist config param of error status codes to swap)
Only adds 2 lines of code !

[alexpetros]

Added the idea we talked about yesterday @alexpetros (whitelist config param of error status codes to swap) Only adds 2 lines of code !

Love this—would suggest you do it in a separate PR, as this one has a lot of new features and configs and will probably go through some revisions.

[Telroshan]

Wouldn't it make more sense to have them all in the same PR ? Since the error codes whitelist cannot exist without this PR, and really adds 2 lines of code to it.
If it's just about patience, no worries I can wait 😆

I would say, if this goes through some revisions, I think it'd better be looked at as a whole, as any required change might impact that whitelist system, so I'd suggest revisions of the whole thing instead of 95% + 5% apart in a second phase, if that makes sense

[alexpetros]

Since the error codes whitelist cannot exist without this PR, and really adds 2 lines of code to it.

Why is this true? We could totally do this separately.

In a broader sense, it lets us put the proper attention on each subset of the interface. A 2-source-line PR that introduces a new config name, documentation, and tests is already a good-sized PR! Once something is in our interface it is in there forever so even the small API changes need scrutiny; such is the nature of maintaining a library.

[Telroshan]

Well, this adds a whitelist to the error swapping system, so if the error swapping system were to be rejected, this whitelist wouldn't make any sense on its own

As you want then, I'll remove the whitelist from this PR
Should I wait until this PR eventually gets merged (if not closed) before making the PR for the whitelist then? Or would you like me to fire it as a second PR right away?

[alexpetros]

I can push a branch and show you what I mean. Also there's no rush because this PR isn't going to be in the upcoming release.

[Telroshan]

If you have the time for it, I'd be glad to have an example, I must admit I'm a bit confused about what you expect me to do!

Yeah I knew that, no worries!

[Telroshan]

Forgot about this, removed the whitelist from the PR @alexpetros

[ethanniser]

is there a reason this ran out of steam?

I have been using the response-targets extension, and found it difficult being forced to use the same hx-swap value for error and success swaps

But this pr looks even better, most people dont need the fine grainedness of response-targets, just success or error

would be happy to help to get this merged if others are still interested

[Telroshan]

It's just that there were (and still are) a lot of PRs to process, so PRs with lots of changes always take some time
Though now that 1.9.6 is out of the way, I think this can be looked at for the next release, thanks for the ping!

[alexpetros]

This PR turned out to be a bit more than we can chew in a single PR. After much discussion, including with @Telroshan, this is how we're going to break it down:

1. Merge Add hx-target-error attribute to response-targets extension #1929. In addition to being a much, much smaller change, I also prefer hx-target-error to hx-error-target, because it extends the existing namespace, and aligns with the hx-target-5xx schema of the response targets extension.
2. Discuss merging the response targets extension into the core, since it's one of the most commonly requested features.
3. Evaluate htmx.config.defaultErrorTarget as a config option in a separate PR.
4. Evaluate expanding the response swapping to customize not just the target, but the swap style (either as part of the extension or the core, depending on the outcome of that discussion)

I want to reiterate that we are much interested in getting better error swapping into the core. It's a pain point for myself, and may others, as demonstrated by the interest in this PR and the parent issue. Core changes are irreversible; barring extreme cases, if an attribute or config options is merged into the core, we're supporting it forever. So having to make this many irreversible changes on PR overwhelmed our bandwidth. I think this path forward will unblock the issue and get us some better error swapping soon.

See #1619 for future updates.