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

Feature: add extensionsToIgnore to withExtensions #3195

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

namzug16
Copy link

Description

extensionsToIgnore has been added to withExtensions which allows developers to filter the extensions within the internal api.

Corresponding issue:
bigskysoftware/htmx-extensions#145

Testing

Please explain how you tested this change manually, and, if applicable, what new tests you added. If
you're making a change to just the website, you can omit this section.

I've manually tested it by creating an extension merge-vals that modifies the parameters through encodeParameters, while being used at the same time with json-enc-custom like so

...
hx-ext="merge-vals,json-enc-custom"
...

the extension implementation:

    encodeParameters: function(xhr, parameters, elt) {
      xhr.overrideMimeType("text/json");

      let mergedVals = api.mergeObjects({}, parameters);

      if (elt.hasAttribute("hx-merge-vals")) {
        ...
        // simplified code
      }

      return JSON.stringify(mergedVals);
    }

The issue with this implementation is that htmx will only use the first extension that modifies the parameters, so json-custom-enc will never be used

a simple fix would to manually call the extensions like so

    encodeParameters: function(xhr, parameters, elt) {
      xhr.overrideMimeType("text/json");

      let mergedVals = api.mergeObjects({}, parameters);

     ...
     // simplified code
     
      api.withExtensions(elt, function(extension) {
        mergedVals = JSON.parse(extension.encodeParameters(xhr, mergedVals, elt))
      })

      return JSON.stringify(mergedVals);
    }

Now, a different issue arises, withExtensions will get all the specified extensions from the element, including merge-vals which will cause an infinity loop

This PR introduces extensionsToIgnore into withExntensions which fixes this issue without the introduction of a breaking change to the logic of encode parameters

    encodeParameters: function(xhr, parameters, elt) {
      xhr.overrideMimeType("text/json");

      let mergedVals = api.mergeObjects({}, parameters);

     ...
     // simplified code
     
      api.withExtensions(elt, function(extension) {
        mergedVals = JSON.parse(extension.encodeParameters(xhr, mergedVals, elt))
      }, ["merge-vals"])

      return JSON.stringify(mergedVals);
    }

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@Telroshan Telroshan added the enhancement New feature or request label Feb 19, 2025
@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants