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

Fix example for responseHandling config via <meta> tag #2715

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

ekzyis
Copy link
Contributor

@ekzyis ekzyis commented Jul 12, 2024

Description

In the docs, the following example is given to configure response handling via the <meta> tag:

<meta name="htmx-config" content='{code:"204", swap: false},   // 204 - No Content by default does nothing, but is not an error
                                  {code:"[23]..", swap: true}, // 200 & 300 responses are non-errors and are swapped
                                  {code:"422", swap: true}, // 422 responses are swapped
                                  {code:"[45]..", swap: false, error:true}, // 400 & 500 responses are not swapped and are errors'>

However, if this is used in a site which loads htmx v0.2.0, the following error is thrown:

htmx.js:2914 SyntaxError: Expected property name or '}' in JSON at position 1 (line 1 column 2)
    at JSON.parse (<anonymous>)
    at parseJSON (htmx.js:816:19)
    at getMetaConfig (htmx.js:4902:14)
    at mergeMetaConfig (htmx.js:4909:24)
    at HTMLDocument.<anonymous> (htmx.js:4917:5)

Fixing all errors like unquoted keys, comments and missing responseHandling key lead me to this config:

<meta
	name="htmx-config"
	content='{
		"responseHandling":[
			{"code":"204", "swap": false},
			{"code":"[23]..", "swap": true},
			{"code":"422", "swap": true},
			{"code":"[45]..", "swap": false, "error":true}
		]
	}'
/>

It's unfortunate that JSON does not support comments but I think having a working example in the documentation is more important.

Corresponding issue: n/a

Testing

I tested this change by copy-pasting the configuration into <head> of my site

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

The current example throws this error:

  htmx.js:2914 SyntaxError: Expected property name or '}' in JSON at position 1 (line 1 column 2)
    at JSON.parse (<anonymous>)
    at parseJSON (htmx.js:816:19)
    at getMetaConfig (htmx.js:4902:14)
    at mergeMetaConfig (htmx.js:4909:24)
    at HTMLDocument.<anonymous> (htmx.js:4917:5)
@Telroshan Telroshan added the documentation Improvements or additions to documentation label Jul 12, 2024
@gemmadlou
Copy link

gemmadlou commented Jul 25, 2024

Hi @ekzyis
I had this problem too and realised I need to format it in JSON. Thanks for the fix. Hope it gets reviewed soon.

But what I failed to understand was I needed to include the responseHandling property on the JSON object, whereas I was just doing this:

<meta name="htmx-config" content='{"code":".*", "swap": true}'>

But this works a treat!

    <meta name="htmx-config" content='{
        "responseHandling":[
            {"code":".*", "swap": true}
        ]
    }' />

@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Jul 25, 2024
@alexpetros
Copy link
Collaborator

alexpetros commented Jul 26, 2024

First of all, great fix, much appreciated.

Would you mind updating it to include the original commented info as an HTML comment above the config? I do think the explanation is necessary there.

@geoffrey-eisenbarth
Copy link
Contributor

One thing that I think is important to specify in the documentation is that if there are any status codes that you don't handle in the config, they won't be swapped. I mentioned this on discord and Carson mentioned that you should include the catch-all case "code": "...", "swap": true

@1cg 1cg merged commit e2b671d into bigskysoftware:master Aug 5, 2024
1 check passed
@ekzyis
Copy link
Contributor Author

ekzyis commented Aug 5, 2024

Oh, sorry for the slow response, I kept forgetting after I read the notifications to do the changes when I got home and now I read the notification that this was merged now.

Would you mind updating it to include the original commented info as an HTML comment above the config? I do think the explanation is necessary there.

I'll add this now and the catch-all case!

@alexpetros
Copy link
Collaborator

Carson just merged it so just open a new PR and tag me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation 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.

6 participants