From 2ded3326e5fdab8211ce816b220a959a200faef2 Mon Sep 17 00:00:00 2001 From: Tanner Reits Date: Mon, 23 Oct 2023 11:12:11 -0400 Subject: [PATCH 1/5] fix(runtime): handle capture style events --- src/runtime/vdom/set-accessor.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/runtime/vdom/set-accessor.ts b/src/runtime/vdom/set-accessor.ts index 8fd9e0e2e67..0cd6c8a296a 100644 --- a/src/runtime/vdom/set-accessor.ts +++ b/src/runtime/vdom/set-accessor.ts @@ -109,11 +109,20 @@ export const setAccessor = ( // except for the first character, we keep the event name case memberName = ln[2] + memberName.slice(3); } - if (oldValue) { - plt.rel(elm, memberName, oldValue, false); - } - if (newValue) { - plt.ael(elm, memberName, newValue, false); + if (oldValue || newValue) { + // Need to account for "capture" events + let capture = false; + if (memberName.endsWith(CAPTURE_EVENT_SUFFIX)) { + capture = true; + memberName = memberName.replace(CAPTURE_EVENT_SUFFIX, ''); + } + + if (oldValue) { + plt.rel(elm, memberName, oldValue, capture); + } + if (newValue) { + plt.ael(elm, memberName, newValue, capture); + } } } else if (BUILD.vdomPropOrAttr) { // Set property if it exists and it's not a SVG @@ -170,3 +179,4 @@ export const setAccessor = ( }; const parseClassListRegex = /\s/; const parseClassList = (value: string | undefined | null): string[] => (!value ? [] : value.split(parseClassListRegex)); +const CAPTURE_EVENT_SUFFIX = 'Capture'; From a507ca71474c47bf69da1ba48f8ccc36c3bb4d52 Mon Sep 17 00:00:00 2001 From: Tanner Reits Date: Mon, 23 Oct 2023 14:55:59 -0400 Subject: [PATCH 2/5] add some more comments for context --- src/runtime/vdom/set-accessor.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/runtime/vdom/set-accessor.ts b/src/runtime/vdom/set-accessor.ts index 0cd6c8a296a..d2bc7743e65 100644 --- a/src/runtime/vdom/set-accessor.ts +++ b/src/runtime/vdom/set-accessor.ts @@ -110,11 +110,14 @@ export const setAccessor = ( memberName = ln[2] + memberName.slice(3); } if (oldValue || newValue) { - // Need to account for "capture" events + // Need to account for "capture" events. + // If the event name ends with "Capture", we'll update the name to remove + // the "Capture" suffix and make sure the event listener is setup to handle the capture event. let capture = false; if (memberName.endsWith(CAPTURE_EVENT_SUFFIX)) { capture = true; - memberName = memberName.replace(CAPTURE_EVENT_SUFFIX, ''); + // Make sure we only replace the last instance of "Capture" + memberName = memberName.replace(new RegExp(CAPTURE_EVENT_SUFFIX + '$'), ''); } if (oldValue) { From 3a662e6989cd478571c79c0398cdf2c234dfc389 Mon Sep 17 00:00:00 2001 From: Tanner Reits Date: Mon, 23 Oct 2023 14:56:23 -0400 Subject: [PATCH 3/5] tests are good to have --- src/runtime/vdom/test/set-accessor.spec.ts | 14 +++++++++ test/karma/test-app/components.d.ts | 13 ++++++++ .../test-app/event-listener-capture/cmp.tsx | 21 +++++++++++++ .../event-listener-capture/index.html | 6 ++++ .../event-listener-capture/karma.spec.ts | 31 +++++++++++++++++++ 5 files changed, 85 insertions(+) create mode 100644 test/karma/test-app/event-listener-capture/cmp.tsx create mode 100644 test/karma/test-app/event-listener-capture/index.html create mode 100644 test/karma/test-app/event-listener-capture/karma.spec.ts diff --git a/src/runtime/vdom/test/set-accessor.spec.ts b/src/runtime/vdom/test/set-accessor.spec.ts index de44d032027..7ba5ec2434c 100644 --- a/src/runtime/vdom/test/set-accessor.spec.ts +++ b/src/runtime/vdom/test/set-accessor.spec.ts @@ -113,6 +113,20 @@ describe('setAccessor for custom elements', () => { expect(addEventSpy).toHaveBeenCalledWith('click', newValue, false); expect(removeEventSpy).not.toHaveBeenCalled(); }); + + it('should add a capture style event listener', () => { + const addEventSpy = jest.spyOn(elm, 'addEventListener'); + const removeEventSpy = jest.spyOn(elm, 'removeEventListener'); + + const newValue = () => { + /**/ + }; + + setAccessor(elm, 'onClickCapture', undefined, newValue, false, 0); + + expect(addEventSpy).toHaveBeenCalledWith('click', newValue, true); + expect(removeEventSpy).not.toHaveBeenCalled(); + }); }); it('should set object property to child', () => { diff --git a/test/karma/test-app/components.d.ts b/test/karma/test-app/components.d.ts index ab733e4b10a..69d8b5f6923 100644 --- a/test/karma/test-app/components.d.ts +++ b/test/karma/test-app/components.d.ts @@ -131,6 +131,8 @@ export namespace Components { } interface EventCustomType { } + interface EventListenerCapture { + } interface ExternalImportA { } interface ExternalImportB { @@ -676,6 +678,12 @@ declare global { prototype: HTMLEventCustomTypeElement; new (): HTMLEventCustomTypeElement; }; + interface HTMLEventListenerCaptureElement extends Components.EventListenerCapture, HTMLStencilElement { + } + var HTMLEventListenerCaptureElement: { + prototype: HTMLEventListenerCaptureElement; + new (): HTMLEventListenerCaptureElement; + }; interface HTMLExternalImportAElement extends Components.ExternalImportA, HTMLStencilElement { } var HTMLExternalImportAElement: { @@ -1355,6 +1363,7 @@ declare global { "esm-import": HTMLEsmImportElement; "event-basic": HTMLEventBasicElement; "event-custom-type": HTMLEventCustomTypeElement; + "event-listener-capture": HTMLEventListenerCaptureElement; "external-import-a": HTMLExternalImportAElement; "external-import-b": HTMLExternalImportBElement; "external-import-c": HTMLExternalImportCElement; @@ -1575,6 +1584,8 @@ declare namespace LocalJSX { interface EventCustomType { "onTestEvent"?: (event: EventCustomTypeCustomEvent) => void; } + interface EventListenerCapture { + } interface ExternalImportA { } interface ExternalImportB { @@ -1856,6 +1867,7 @@ declare namespace LocalJSX { "esm-import": EsmImport; "event-basic": EventBasic; "event-custom-type": EventCustomType; + "event-listener-capture": EventListenerCapture; "external-import-a": ExternalImportA; "external-import-b": ExternalImportB; "external-import-c": ExternalImportC; @@ -2002,6 +2014,7 @@ declare module "@stencil/core" { "esm-import": LocalJSX.EsmImport & JSXBase.HTMLAttributes; "event-basic": LocalJSX.EventBasic & JSXBase.HTMLAttributes; "event-custom-type": LocalJSX.EventCustomType & JSXBase.HTMLAttributes; + "event-listener-capture": LocalJSX.EventListenerCapture & JSXBase.HTMLAttributes; "external-import-a": LocalJSX.ExternalImportA & JSXBase.HTMLAttributes; "external-import-b": LocalJSX.ExternalImportB & JSXBase.HTMLAttributes; "external-import-c": LocalJSX.ExternalImportC & JSXBase.HTMLAttributes; diff --git a/test/karma/test-app/event-listener-capture/cmp.tsx b/test/karma/test-app/event-listener-capture/cmp.tsx new file mode 100644 index 00000000000..2093c9af6e5 --- /dev/null +++ b/test/karma/test-app/event-listener-capture/cmp.tsx @@ -0,0 +1,21 @@ +import { Component, h, State } from '@stencil/core'; + +@Component({ + tag: 'event-listener-capture', +}) +export class EventListenerCapture { + @State() counter = 0; + + render() { + return ( +
+

Click the text below to trigger a capture style event

+
+

this.counter++}> + Clicked: {this.counter} time(s) +

+
+
+ ); + } +} diff --git a/test/karma/test-app/event-listener-capture/index.html b/test/karma/test-app/event-listener-capture/index.html new file mode 100644 index 00000000000..61d31e9c1c8 --- /dev/null +++ b/test/karma/test-app/event-listener-capture/index.html @@ -0,0 +1,6 @@ + + + + + + diff --git a/test/karma/test-app/event-listener-capture/karma.spec.ts b/test/karma/test-app/event-listener-capture/karma.spec.ts new file mode 100644 index 00000000000..c988fdbee50 --- /dev/null +++ b/test/karma/test-app/event-listener-capture/karma.spec.ts @@ -0,0 +1,31 @@ +import { setupDomTests, waitForChanges } from '../util'; + +describe('event-listener-capture', function () { + const { setupDom, tearDownDom } = setupDomTests(document); + + let app: HTMLElement | undefined; + let host: HTMLElement | undefined; + + beforeEach(async () => { + app = await setupDom('/event-listener-capture/index.html'); + host = app.querySelector('event-listener-capture'); + }); + + afterEach(tearDownDom); + + it('should render', () => { + expect(host).toBeDefined(); + }); + + it('should increment counter on click', async () => { + const counter = host.querySelector('#counter'); + expect(counter.textContent).toBe('0'); + + const p = host.querySelector('#incrementer') as HTMLParagraphElement; + expect(p).toBeDefined(); + p.click(); + await waitForChanges(); + + expect(counter.textContent).toBe('1'); + }); +}); From 5439860b193fd45ae453405447e9db108689a333 Mon Sep 17 00:00:00 2001 From: Tanner Reits Date: Mon, 23 Oct 2023 15:11:31 -0400 Subject: [PATCH 4/5] forgot a test for removing event listeners --- src/runtime/vdom/test/set-accessor.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/runtime/vdom/test/set-accessor.spec.ts b/src/runtime/vdom/test/set-accessor.spec.ts index 7ba5ec2434c..35c825648aa 100644 --- a/src/runtime/vdom/test/set-accessor.spec.ts +++ b/src/runtime/vdom/test/set-accessor.spec.ts @@ -127,6 +127,22 @@ describe('setAccessor for custom elements', () => { expect(addEventSpy).toHaveBeenCalledWith('click', newValue, true); expect(removeEventSpy).not.toHaveBeenCalled(); }); + + it('should remove a capture style event listener', () => { + const addEventSpy = jest.spyOn(elm, 'addEventListener'); + const removeEventSpy = jest.spyOn(elm, 'removeEventListener'); + + const orgValue = () => { + /**/ + }; + + setAccessor(elm, 'onClickCapture', undefined, orgValue, false, 0); + setAccessor(elm, 'onClickCapture', orgValue, undefined, false, 0); + + expect(addEventSpy).toHaveBeenCalledTimes(1); + expect(addEventSpy).toHaveBeenCalledWith('click', orgValue, true); + expect(removeEventSpy).toHaveBeenCalledWith('click', orgValue, true); + }); }); it('should set object property to child', () => { From 604b4f928ed370e5aecdbcb70fc517c7174be3ab Mon Sep 17 00:00:00 2001 From: Tanner Reits Date: Wed, 25 Oct 2023 20:16:35 -0400 Subject: [PATCH 5/5] PR feedback --- src/runtime/vdom/set-accessor.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/runtime/vdom/set-accessor.ts b/src/runtime/vdom/set-accessor.ts index d2bc7743e65..bfaa87d4158 100644 --- a/src/runtime/vdom/set-accessor.ts +++ b/src/runtime/vdom/set-accessor.ts @@ -113,12 +113,9 @@ export const setAccessor = ( // Need to account for "capture" events. // If the event name ends with "Capture", we'll update the name to remove // the "Capture" suffix and make sure the event listener is setup to handle the capture event. - let capture = false; - if (memberName.endsWith(CAPTURE_EVENT_SUFFIX)) { - capture = true; - // Make sure we only replace the last instance of "Capture" - memberName = memberName.replace(new RegExp(CAPTURE_EVENT_SUFFIX + '$'), ''); - } + const capture = memberName.endsWith(CAPTURE_EVENT_SUFFIX); + // Make sure we only replace the last instance of "Capture" + memberName = memberName.replace(CAPTURE_EVENT_REGEX, ''); if (oldValue) { plt.rel(elm, memberName, oldValue, capture); @@ -183,3 +180,4 @@ export const setAccessor = ( const parseClassListRegex = /\s/; const parseClassList = (value: string | undefined | null): string[] => (!value ? [] : value.split(parseClassListRegex)); const CAPTURE_EVENT_SUFFIX = 'Capture'; +const CAPTURE_EVENT_REGEX = new RegExp(CAPTURE_EVENT_SUFFIX + '$');