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

Provide correct context to issueAjaxRequest #2770

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

Conversation

patreeceeo
Copy link

I believe this fixes #2147

Description

Instead of passing the element that the request was triggered on, ajax is passing null which causes issueAjaxRequest to default to the BODY element. When multiple requests are queued at once, htmx thinks they're all associated with BODY and so it ignores or limits the subsequent simultaneous requests.

Corresponding issue: #2147

Testing

I've used this change in my own project. Also ran htmx's own tests.

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 bug Something isn't working label Jul 30, 2024
@Telroshan
Copy link
Collaborator

Telroshan commented Jul 30, 2024

Hey, could you add a test case for this?
As we mock timeouts in the test suite and manually trigger server responses, I think you should be able to replicate the situation where you have multiple requests at the same time and reproduce the referenced issue.
Just to make sure that the issue is indeed happening as described, then is indeed fixed by this PR (and prevent potential future regressions)!

@Telroshan Telroshan added the needs test PR needs a test label Jul 30, 2024
@patreeceeo
Copy link
Author

Sure, I should be able to do that next week.

@patreeceeo
Copy link
Author

patreeceeo commented Aug 6, 2024

Working on that test but having difficulty reproducing the bug right now. I noticed that Sinon seems to cause issueAjaxRequest to be called an extra time, and perhaps that's why this test is passing?

  it('ajax api works concurrently', function() {
    this.server.respondWith('GET', '/test', function(xhr) {
      xhr.respond(200, {}, 'foo')
    })
    this.server.respondWith('GET', '/test2', function(xhr) {
      xhr.respond(200, {}, 'foo 2')
    })
    var div = make('<div></div>')
    var div2 = make('<div></div>')
    console.log('1st request')
    htmx.ajax('GET', '/test', div)
    console.log('2d request')
    htmx.ajax('GET', '/test2', div2)
    this.server.respond()
    console.log('1st response')
    this.server.respond()
    console.log('2nd response')
    div.innerHTML.should.equal('foo')
    div2.innerHTML.should.equal('foo 2')
    console.log('test over');
  })

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs test PR needs a test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants