htmx icon indicating copy to clipboard operation
htmx copied to clipboard

Provide correct context to issueAjaxRequest

Open patreeceeo opened this issue 1 year ago • 3 comments

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

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

patreeceeo avatar Jul 29 '24 19:07 patreeceeo

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 avatar Jul 30 '24 08:07 Telroshan

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

patreeceeo avatar Jul 30 '24 22:07 patreeceeo

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

patreeceeo avatar Aug 06 '24 22:08 patreeceeo

@patreeceeo
Hi, pls see this response: https://github.com/bigskysoftware/htmx/issues/2147#issuecomment-2296405566

I think we don't need any test.

DemaPy avatar Jan 01 '25 20:01 DemaPy