From 86f0e9437b295c9bc097470c0aa737f6f298b1a8 Mon Sep 17 00:00:00 2001 From: Joerg Sonnenberger Date: Wed, 11 Sep 2024 01:43:56 +0200 Subject: [PATCH 1/3] Adjust hx-trigger's changed modifier for multiple sources The `changed` trigger modifier can see different event targets, either due to the `from` modifier or event bubbling. The existing behavior trigger only for one node (`from` modifier) or inconsistently (bubbling). Use a nested weak map to keep track of the last value per distinguished (trigger specification, event target node) pair. The weak map ensures that Garbage Collection can still recycle the nodes. If a event target was not seen via `from`, it is assumed changed for the first time the trigger is hit. --- src/htmx.js | 26 ++++++++++---------- test/attributes/hx-trigger.js | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/htmx.js b/src/htmx.js index 41ae9b382..d53ec3f10 100644 --- a/src/htmx.js +++ b/src/htmx.js @@ -695,7 +695,7 @@ var htmx = (function() { * @property {boolean} [triggeredOnce] * @property {number} [delayed] * @property {number|null} [throttle] - * @property {string} [lastValue] + * @property {WeakMap>} [lastValue] * @property {boolean} [loaded] * @property {string} [path] * @property {string} [verb] @@ -2407,10 +2407,15 @@ var htmx = (function() { } // store the initial values of the elements, so we can tell if they change if (triggerSpec.changed) { + if (!('lastValue' in elementData)) { + elementData.lastValue = new WeakMap() + } eltsToListenOn.forEach(function(eltToListenOn) { - const eltToListenOnData = getInternalData(eltToListenOn) + if (!elementData.lastValue.has(triggerSpec)) { + elementData.lastValue.set(triggerSpec, new WeakMap()) + } // @ts-ignore value will be undefined for non-input elements, which is fine - eltToListenOnData.lastValue = eltToListenOn.value + elementData.lastValue.get(triggerSpec).set(eltToListenOn, eltToListenOn.value) }) } forEach(eltsToListenOn, function(eltToListenOn) { @@ -2452,13 +2457,14 @@ var htmx = (function() { } } if (triggerSpec.changed) { - const eltToListenOnData = getInternalData(eltToListenOn) + const node = event.target // @ts-ignore value will be undefined for non-input elements, which is fine - const value = eltToListenOn.value - if (eltToListenOnData.lastValue === value) { + const value = node.value + const lastValue = elementData.lastValue.get(triggerSpec) + if (lastValue.has(node) && lastValue.get(node) === value) { return } - eltToListenOnData.lastValue = value + lastValue.set(node, value) } if (elementData.delayed) { clearTimeout(elementData.delayed) @@ -2836,12 +2842,6 @@ var htmx = (function() { triggerEvent(elt, 'htmx:beforeProcessNode') - // @ts-ignore value will be undefined for non-input elements, which is fine - if (elt.value) { - // @ts-ignore - nodeData.lastValue = elt.value - } - const triggerSpecs = getTriggerSpecs(elt) const hasExplicitHttpAction = processVerbs(elt, nodeData, triggerSpecs) diff --git a/test/attributes/hx-trigger.js b/test/attributes/hx-trigger.js index a83a29e16..2c622eb78 100644 --- a/test/attributes/hx-trigger.js +++ b/test/attributes/hx-trigger.js @@ -64,6 +64,7 @@ describe('hx-trigger attribute', function() { div.innerHTML.should.equal('Requests: 1') }) + // This test and the next one should be kept in sync. it('changed modifier works along from clause with two inputs', function() { var requests = 0 this.server.respondWith('GET', '/test', function(xhr) { @@ -106,6 +107,50 @@ describe('hx-trigger attribute', function() { div.innerHTML.should.equal('Requests: 2') }) + // This test and the previous one should be kept in sync. + it('changed modifier counts each triggerspec separately', function() { + var requests = 0 + this.server.respondWith('GET', '/test', function(xhr) { + requests++ + xhr.respond(200, {}, 'Requests: ' + requests) + }) + var input1 = make('') + var input2 = make('') + make('
') + make('
') + var div = make('
') + + input1.click() + this.server.respond() + div.innerHTML.should.equal('') + input2.click() + this.server.respond() + div.innerHTML.should.equal('') + + input1.value = 'bar' + input2.click() + this.server.respond() + div.innerHTML.should.equal('') + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 2') + + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 2') + input2.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 2') + + input2.value = 'foo' + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 2') + input2.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 4') + }) + it('once modifier works', function() { var requests = 0 this.server.respondWith('GET', '/test', function(xhr) { From 1dfbe66255bd7803c4c4db782b30d43db4a12eb2 Mon Sep 17 00:00:00 2001 From: Joerg Sonnenberger Date: Wed, 11 Sep 2024 02:12:05 +0200 Subject: [PATCH 2/3] Add test case for separate triggers with changed modifier --- test/attributes/hx-trigger.js | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/attributes/hx-trigger.js b/test/attributes/hx-trigger.js index 2c622eb78..f2c1211f8 100644 --- a/test/attributes/hx-trigger.js +++ b/test/attributes/hx-trigger.js @@ -151,6 +151,48 @@ describe('hx-trigger attribute', function() { div.innerHTML.should.equal('Requests: 4') }) + it('separate changed modifier works along from clause with two inputs', function() { + var requests = 0 + this.server.respondWith('GET', '/test', function(xhr) { + requests++ + xhr.respond(200, {}, 'Requests: ' + requests) + }) + var input1 = make('') + var input2 = make('') + make('
') + var div = make('
') + + input1.click() + this.server.respond() + div.innerHTML.should.equal('') + input2.click() + this.server.respond() + div.innerHTML.should.equal('') + + input1.value = 'bar' + input2.click() + this.server.respond() + div.innerHTML.should.equal('') + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 1') + + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 1') + input2.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 1') + + input2.value = 'foo' + input1.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 1') + input2.click() + this.server.respond() + div.innerHTML.should.equal('Requests: 2') + }) + it('once modifier works', function() { var requests = 0 this.server.respondWith('GET', '/test', function(xhr) { From b19cb434a8ecb200258075774fde0d165a021c1f Mon Sep 17 00:00:00 2001 From: Joerg Sonnenberger Date: Wed, 11 Sep 2024 15:48:17 +0200 Subject: [PATCH 3/3] Ensure that each trigger spec has independent delay slots --- src/htmx.js | 51 +++++++++++++++++++++++++++------ test/attributes/hx-trigger.js | 54 +++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/htmx.js b/src/htmx.js index d53ec3f10..9e2d3f7ec 100644 --- a/src/htmx.js +++ b/src/htmx.js @@ -693,9 +693,10 @@ var htmx = (function() { * @property {ListenerInfo[]} [listenerInfos] * @property {boolean} [cancelled] * @property {boolean} [triggeredOnce] - * @property {number} [delayed] + * @property {WeakMap} [delayed] * @property {number|null} [throttle] * @property {WeakMap>} [lastValue] + * @property {WeakMap>} [pendingValue] * @property {boolean} [loaded] * @property {string} [path] * @property {string} [verb] @@ -2405,6 +2406,14 @@ var htmx = (function() { } else { eltsToListenOn = [elt] } + if (triggerSpec.delay > 0) { + if (!('delayed' in elementData)) { + elementData.delayed = new WeakMap() + } + if (!('pendingValue' in elementData)) { + elementData.pendingValue = new WeakMap() + } + } // store the initial values of the elements, so we can tell if they change if (triggerSpec.changed) { if (!('lastValue' in elementData)) { @@ -2461,13 +2470,22 @@ var htmx = (function() { // @ts-ignore value will be undefined for non-input elements, which is fine const value = node.value const lastValue = elementData.lastValue.get(triggerSpec) - if (lastValue.has(node) && lastValue.get(node) === value) { - return + if (triggerSpec.delay > 0) { + if (!elementData.pendingValue.has(triggerSpec)) { + elementData.pendingValue.set(triggerSpec, new Map()) + } + const pendingValue = elementData.pendingValue.get(triggerSpec) + pendingValue.set(node, value) + } else { + if (lastValue.has(node) && lastValue.get(node) === value) { + return + } + lastValue.set(node, value) } - lastValue.set(node, value) } - if (elementData.delayed) { - clearTimeout(elementData.delayed) + if (elementData.delayed && elementData.delayed.has(triggerSpec)) { + clearTimeout(elementData.delayed.get(triggerSpec)) + elementData.delayed.delete(triggerSpec) } if (elementData.throttle) { return @@ -2482,10 +2500,27 @@ var htmx = (function() { }, triggerSpec.throttle) } } else if (triggerSpec.delay > 0) { - elementData.delayed = getWindow().setTimeout(function() { + elementData.delayed.set(triggerSpec, getWindow().setTimeout(function() { + elementData.delayed.delete(triggerSpec) + if (triggerSpec.changed) { + const lastValue = elementData.lastValue.get(triggerSpec) || new WeakMap() + const pendingValue = elementData.pendingValue.get(triggerSpec) || new Map() + elementData.pendingValue.set(triggerSpec, new Map()) + + let changed = false + pendingValue.forEach((new_value, new_node) => { + if (lastValue.get(new_node) !== new_value) { + changed = true + lastValue.set(new_node, new_value) + } + }) + if (!changed) { + return + } + } triggerEvent(elt, 'htmx:trigger') handler(elt, evt) - }, triggerSpec.delay) + }, triggerSpec.delay)) } else { triggerEvent(elt, 'htmx:trigger') handler(elt, evt) diff --git a/test/attributes/hx-trigger.js b/test/attributes/hx-trigger.js index f2c1211f8..84bfd5088 100644 --- a/test/attributes/hx-trigger.js +++ b/test/attributes/hx-trigger.js @@ -810,6 +810,60 @@ describe('hx-trigger attribute', function() { }, 50) }) + it('two delays on the same node are independent', function(done) { + var requests = 0 + var server = this.server + this.server.respondWith('GET', '/test', function(xhr) { + requests++ + xhr.respond(200, {}, 'Requests: ' + requests) + }) + this.server.respondWith('GET', '/bar', 'bar') + var button = make('') + var div = make("
") + + div.click() + button.click() + this.server.respond() + div.innerText.should.equal('') + + setTimeout(function() { + server.respond() + div.innerText.should.equal('Requests: 1') + + setTimeout(function() { + server.respond() + div.innerText.should.equal('Requests: 2') + + done() + }, 50) + }, 20) + }) + + it('delay for multiple nodes are shared', function(done) { + var requests = 0 + var server = this.server + this.server.respondWith('GET', '/test', function(xhr) { + requests++ + xhr.respond(200, {}, 'Requests: ' + requests) + }) + this.server.respondWith('GET', '/bar', 'bar') + var button1 = make('') + var button2 = make('') + var div = make("
") + + button1.click() + button2.click() + this.server.respond() + div.innerText.should.equal('') + + setTimeout(function() { + server.respond() + div.innerText.should.equal('Requests: 1') + + done() + }, 20) + }) + it('A 0 delay does not delay the request', function(done) { var requests = 0 this.server.respondWith('GET', '/test', function(xhr) {