Skip to content

Commit 972ce3f

Browse files
authored
feat(): support mixed case events (#1856)
1 parent 55bdd7d commit 972ce3f

File tree

10 files changed

+153
-47
lines changed

10 files changed

+153
-47
lines changed

src/client/client-host-ref.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,4 @@ export const registerHost = (elm: d.HostElement) => {
2727
});
2828
}
2929
};
30-
3130
export const isMemberInElement = (elm: any, memberName: string) => memberName in elm;

src/mock-doc/attribute.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ export class MockAttr {
105105
private _value: string;
106106
private _namespaceURI: string;
107107

108-
constructor(attrName: string, attrValue = '', namespaceURI: string = null) {
108+
constructor(attrName: string, attrValue: string, namespaceURI: string = null) {
109109
this._name = attrName;
110-
this._value = String(attrValue || '');
110+
this._value = String(attrValue);
111111
this._namespaceURI = namespaceURI;
112112
}
113113

@@ -122,7 +122,7 @@ export class MockAttr {
122122
return this._value;
123123
}
124124
set value(value) {
125-
this._value = String(value || '');
125+
this._value = String(value);
126126
}
127127

128128
get nodeName() {
@@ -136,7 +136,7 @@ export class MockAttr {
136136
return this._value;
137137
}
138138
set nodeValue(value) {
139-
this._value = String(value || '');
139+
this._value = String(value);
140140
}
141141

142142
get namespaceURI() {

src/mock-doc/document.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,11 @@ export class MockDocument extends MockHTMLElement {
153153
}
154154

155155
createAttribute(attrName: string) {
156-
return new MockAttr(attrName.toLowerCase());
156+
return new MockAttr(attrName.toLowerCase(), '');
157157
}
158158

159159
createAttributeNS(namespaceURI: string, attrName: string) {
160-
return new MockAttr(attrName, undefined, namespaceURI);
160+
return new MockAttr(attrName, '', namespaceURI);
161161
}
162162

163163
createElement(tagName: string) {

src/mock-doc/node.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,20 +473,20 @@ export class MockElement extends MockNode {
473473
if (attr != null) {
474474
if (checkAttrChanged === true) {
475475
const oldValue = attr.value;
476-
attr.value = String(value);
476+
attr.value = value;
477477

478478
if (oldValue !== attr.value) {
479479
attributeChanged(this, attr.name, oldValue, attr.value);
480480
}
481481
} else {
482-
attr.value = String(value);
482+
attr.value = value;
483483
}
484484

485485
} else {
486486
if (attributes.caseInsensitive) {
487487
attrName = attrName.toLowerCase();
488488
}
489-
attr = new MockAttr(attrName, String(value));
489+
attr = new MockAttr(attrName, value);
490490
attributes.items.push(attr);
491491

492492
if (checkAttrChanged === true) {
@@ -504,13 +504,13 @@ export class MockElement extends MockNode {
504504
if (attr != null) {
505505
if (checkAttrChanged === true) {
506506
const oldValue = attr.value;
507-
attr.value = String(value);
507+
attr.value = value;
508508

509509
if (oldValue !== attr.value) {
510510
attributeChanged(this, attr.name, oldValue, attr.value);
511511
}
512512
} else {
513-
attr.value = String(value);
513+
attr.value = value;
514514
}
515515

516516
} else {

src/mock-doc/test/element.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,25 @@ describe('element', () => {
233233
expect(element).toEqualHtml(`<div prop1=\"null\" prop2=\"undefined\" prop3=\"0\" prop4=\"1\" prop5=\"hola\" prop6></div>`);
234234
});
235235

236+
it('should cast attributeNS values to string', () => {
237+
const element = new MockHTMLElement(doc, 'div');
238+
element.setAttributeNS(null, 'prop1', null);
239+
element.setAttributeNS(null, 'prop2', undefined);
240+
element.setAttributeNS(null, 'prop3', 0);
241+
element.setAttributeNS(null, 'prop4', 1);
242+
element.setAttributeNS(null, 'prop5', 'hola');
243+
element.setAttributeNS(null, 'prop6', '');
244+
245+
expect(element.getAttribute('prop1')).toBe('null');
246+
expect(element.getAttribute('prop2')).toBe('undefined');
247+
expect(element.getAttribute('prop3')).toBe('0');
248+
expect(element.getAttribute('prop4')).toBe('1');
249+
expect(element.getAttribute('prop5')).toBe('hola');
250+
expect(element.getAttribute('prop6')).toBe('');
251+
252+
expect(element).toEqualHtml(`<div prop1=\"null\" prop2=\"undefined\" prop3=\"0\" prop4=\"1\" prop5=\"hola\" prop6></div>`);
253+
});
254+
236255
it('attributes are case insensible in HTMLElement', () => {
237256
const element = new MockHTMLElement(doc, 'div');
238257

src/runtime/client-hydrate.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as d from '../declarations';
22
import { BUILD } from '@build-conditionals';
33
import { CONTENT_REF_ID, HYDRATE_CHILD_ID, HYDRATE_ID, NODE_TYPE, ORG_LOCATION_ID, SLOT_NODE_ID, TEXT_NODE_ID } from './runtime-constants';
44
import { doc, plt } from '@platform';
5-
import { toLowerCase } from '@utils';
65

76

87
export const initializeClientHydrate = (hostElm: d.HostElement, tagName: string, hostId: string, hostRef: d.HostRef) => {
@@ -86,7 +85,7 @@ const clientHydrate = (
8685
$nodeId$: childIdSplt[1],
8786
$depth$: childIdSplt[2],
8887
$index$: childIdSplt[3],
89-
$tag$: toLowerCase(node.tagName),
88+
$tag$: node.tagName.toLowerCase(),
9089
$elm$: node
9190
};
9291

src/runtime/test/jsx.spec.tsx

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { Component, Prop, h } from '@stencil/core';
1+
import { Component, Prop, h, Host, State } from '@stencil/core';
22
import { newSpecPage } from '@stencil/core/testing';
3+
import { CmpA } from 'fixtures/cmp-a';
34

45

56
describe('jsx', () => {
@@ -33,4 +34,81 @@ describe('jsx', () => {
3334
</cmp-a>
3435
`);
3536
});
37+
38+
describe('event', () => {
39+
@Component({ tag: 'cmp-a'})
40+
class CmpA {
41+
@State() lastEvent: any;
42+
render() {
43+
return (
44+
<Host
45+
onClick={() => this.lastEvent = 'onClick'}
46+
on-Click={() => this.lastEvent = 'on-Click'}
47+
on-scroll={() => this.lastEvent = 'on-scroll'}
48+
onIonChange={() => this.lastEvent = 'onIonChange'}
49+
on-IonChange={() => this.lastEvent = 'on-IonChange'}
50+
on-ALLCAPS={() => this.lastEvent = 'on-ALLCAPS'}
51+
>{this.lastEvent}</Host>
52+
);
53+
}
54+
}
55+
56+
it('click', async () => {
57+
const { root, waitForChanges } = await newSpecPage({
58+
components: [CmpA],
59+
html: `<cmp-a></cmp-a>`
60+
});
61+
root.dispatchEvent(new CustomEvent('click'));
62+
await waitForChanges();
63+
expect(root.textContent).toBe('onClick');
64+
});
65+
66+
it('Click', async () => {
67+
const { root, waitForChanges } = await newSpecPage({
68+
components: [CmpA],
69+
html: `<cmp-a></cmp-a>`
70+
});
71+
root.dispatchEvent(new CustomEvent('Click'));
72+
await waitForChanges();
73+
expect(root.textContent).toBe('on-Click');
74+
});
75+
76+
it('scroll', async () => {
77+
const { root, waitForChanges } = await newSpecPage({
78+
components: [CmpA],
79+
html: `<cmp-a></cmp-a>`
80+
});
81+
root.dispatchEvent(new CustomEvent('scroll'));
82+
await waitForChanges();
83+
expect(root.textContent).toBe('on-scroll');
84+
});
85+
it('ionChange', async () => {
86+
const { root, waitForChanges } = await newSpecPage({
87+
components: [CmpA],
88+
html: `<cmp-a></cmp-a>`
89+
});
90+
root.dispatchEvent(new CustomEvent('ionChange'));
91+
await waitForChanges();
92+
expect(root.textContent).toBe('onIonChange');
93+
});
94+
it('IonChange', async () => {
95+
const { root, waitForChanges } = await newSpecPage({
96+
components: [CmpA],
97+
html: `<cmp-a></cmp-a>`
98+
});
99+
root.dispatchEvent(new CustomEvent('IonChange'));
100+
await waitForChanges();
101+
expect(root.textContent).toBe('on-IonChange');
102+
});
103+
it('ALLCAPS', async () => {
104+
const { root, waitForChanges } = await newSpecPage({
105+
components: [CmpA],
106+
html: `<cmp-a></cmp-a>`
107+
});
108+
root.dispatchEvent(new CustomEvent('ALLCAPS'));
109+
await waitForChanges();
110+
expect(root.textContent).toBe('on-ALLCAPS');
111+
});
112+
});
113+
36114
});

src/runtime/vdom/set-accessor.ts

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,20 @@
99

1010
import { BUILD } from '@build-conditionals';
1111
import { isMemberInElement, plt } from '@platform';
12-
import { isComplexType, toLowerCase } from '@utils';
12+
import { isComplexType } from '@utils';
1313
import { VNODE_FLAGS, XLINK_NS } from '../runtime-constants';
1414

1515
export const setAccessor = (elm: HTMLElement, memberName: string, oldValue: any, newValue: any, isSvg: boolean, flags: number) => {
1616
if (oldValue === newValue) {
1717
return;
1818
}
19+
let isProp = isMemberInElement(elm, memberName);
20+
let ln = memberName.toLowerCase();
1921
if (BUILD.vdomClass && memberName === 'class') {
2022
const classList = elm.classList;
2123
parseClassList(oldValue).forEach(cls => classList.remove(cls));
2224
parseClassList(newValue).forEach(cls => classList.add(cls));
25+
2326
} else if (BUILD.vdomStyle && memberName === 'style') {
2427
// update style attribute, css properties and values
2528
if (BUILD.updatable) {
@@ -52,26 +55,34 @@ export const setAccessor = (elm: HTMLElement, memberName: string, oldValue: any,
5255
newValue(elm);
5356
}
5457

55-
} else if (BUILD.vdomListener && memberName.startsWith('on') && !isMemberInElement(elm, memberName)) {
58+
} else if (BUILD.vdomListener && !isProp && memberName[0] === 'o' && memberName[1] === 'n') {
5659
// Event Handlers
5760
// so if the member name starts with "on" and the 3rd characters is
5861
// a capital letter, and it's not already a member on the element,
5962
// then we're assuming it's an event listener
60-
61-
if (isMemberInElement(elm, toLowerCase(memberName))) {
63+
if (memberName[2] === '-') {
64+
// on- prefixed events
65+
// allows to be explicit about the dom event to listen without any magic
66+
// under the hood:
67+
// <my-cmp on-click> // listens for "click"
68+
// <my-cmp on-Click> // listens for "Click"
69+
// <my-cmp on-ionChange> // listens for "ionChange"
70+
// <my-cmp on-EVENTS> // listens for "EVENTS"
71+
memberName = memberName.substr(3);
72+
} else if (isMemberInElement(elm, ln)) {
6273
// standard event
6374
// the JSX attribute could have been "onMouseOver" and the
6475
// member name "onmouseover" is on the element's prototype
6576
// so let's add the listener "mouseover", which is all lowercased
66-
memberName = toLowerCase(memberName.substring(2));
77+
memberName = ln.substr(2);
6778

6879
} else {
6980
// custom event
7081
// the JSX attribute could have been "onMyCustomEvent"
7182
// so let's trim off the "on" prefix and lowercase the first character
7283
// and add the listener "myCustomEvent"
7384
// except for the first character, we keep the event name case
74-
memberName = toLowerCase(memberName[2]) + memberName.substring(3);
85+
memberName = ln[2] + memberName.substr(3);
7586
}
7687
if (oldValue) {
7788
plt.rel(elm, memberName, oldValue, false);
@@ -82,8 +93,18 @@ export const setAccessor = (elm: HTMLElement, memberName: string, oldValue: any,
8293

8394
} else {
8495
// Set property if it exists and it's not a SVG
85-
const isProp = isMemberInElement(elm, memberName);
8696
const isComplex = isComplexType(newValue);
97+
98+
/**
99+
* Need to manually update attribute if:
100+
* - memberName is not an attribute
101+
* - if we are rendering the host element in order to reflect attribute
102+
* - if it's a SVG, since properties might not work in <svg>
103+
* - if the newValue is null/undefined or 'false'.
104+
*/
105+
const namespace = BUILD.svg && isSvg && (ln !== (ln = ln.replace(/^xlink\:?/, '')))
106+
? XLINK_NS
107+
: null;
87108
if ((isProp || (isComplex && newValue !== null)) && !isSvg) {
88109
try {
89110
if (!elm.tagName.includes('-')) {
@@ -99,28 +120,11 @@ export const setAccessor = (elm: HTMLElement, memberName: string, oldValue: any,
99120
} catch (e) {}
100121
}
101122

102-
103-
/**
104-
* Need to manually update attribute if:
105-
* - memberName is not an attribute
106-
* - if we are rendering the host element in order to reflect attribute
107-
* - if it's a SVG, since properties might not work in <svg>
108-
* - if the newValue is null/undefined or 'false'.
109-
*/
110-
const isXlinkNs = BUILD.svg && isSvg && (memberName !== (memberName = memberName.replace(/^xlink\:?/, ''))) ? true : false;
111123
if (newValue == null || newValue === false) {
112-
if (isXlinkNs) {
113-
elm.removeAttributeNS(XLINK_NS, toLowerCase(memberName));
114-
} else {
115-
elm.removeAttribute(memberName);
116-
}
124+
elm.removeAttributeNS(namespace, ln);
117125
} else if ((!isProp || (flags & VNODE_FLAGS.isHost) || isSvg) && !isComplex) {
118-
newValue = newValue === true ? '' : newValue.toString();
119-
if (isXlinkNs) {
120-
elm.setAttributeNS(XLINK_NS, toLowerCase(memberName), newValue);
121-
} else {
122-
elm.setAttribute(memberName, newValue);
123-
}
126+
newValue = newValue === true ? '' : newValue;
127+
elm.setAttributeNS(namespace, ln, newValue);
124128
}
125129
}
126130
};

src/runtime/vdom/test/attributes.spec.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ describe('attributes', () => {
3737
expect(hostElm.getAttributeNS(XLINK_NS, 'href')).toEqual('#foo');
3838
});
3939

40+
41+
it('are set correctly when namespaced (2)', () => {
42+
const vnode1 = h('svg', { 'xlinkHref': '#foo' });
43+
patch(vnode0, vnode1);
44+
expect(hostElm.getAttributeNS(XLINK_NS, 'href')).toEqual('#foo');
45+
});
46+
4047
it('should not touch class nor id fields', () => {
4148
hostElm = document.createElement('div');
4249
hostElm.id = 'myId';

src/runtime/vdom/vdom-render.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
import * as d from '../../declarations';
1010
import { BUILD } from '@build-conditionals';
11-
import { CMP_FLAGS, SVG_NS, isDef, toLowerCase } from '@utils';
11+
import { CMP_FLAGS, SVG_NS, isDef } from '@utils';
1212
import { consoleError, doc, plt, supportsShadowDom } from '@platform';
1313
import { h, isHost } from './h';
1414
import { NODE_TYPE, PLATFORM_FLAGS, VNODE_FLAGS } from '../runtime-constants';
@@ -67,7 +67,7 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
6767

6868
} else if (BUILD.slotRelocation && newVNode.$flags$ & VNODE_FLAGS.isSlotReference) {
6969
// create a slot reference node
70-
newVNode.$elm$ = (BUILD.isDebug || BUILD.hydrateServerSide) ? doc.createComment(`slot-reference:${hostTagName}`) : doc.createTextNode('') as any;
70+
newVNode.$elm$ = (BUILD.isDebug || BUILD.hydrateServerSide) ? doc.createComment(`slot-reference:${hostTagName.toLowerCase()}`) : doc.createTextNode('') as any;
7171

7272
} else {
7373
// create element
@@ -184,7 +184,7 @@ const addVnodes = (
184184
) => {
185185
let containerElm = ((BUILD.slotRelocation && parentElm['s-cr'] && parentElm['s-cr'].parentNode) || parentElm) as any;
186186
let childNode: Node;
187-
if (BUILD.shadowDom && (containerElm as any).shadowRoot && toLowerCase(containerElm.tagName) === hostTagName) {
187+
if (BUILD.shadowDom && (containerElm as any).shadowRoot && containerElm.tagName === hostTagName) {
188188
containerElm = (containerElm as any).shadowRoot;
189189
}
190190

@@ -568,11 +568,11 @@ interface RelocateNode {
568568
}
569569

570570
export const renderVdom = (hostElm: d.HostElement, hostRef: d.HostRef, cmpMeta: d.ComponentRuntimeMeta, renderFnResults: d.VNode | d.VNode[]) => {
571-
hostTagName = toLowerCase(hostElm.tagName);
571+
hostTagName = hostElm.tagName;
572572
// <Host> runtime check
573573
if (BUILD.isDev && Array.isArray(renderFnResults) && renderFnResults.some(isHost)) {
574574
throw new Error(`The <Host> must be the single root component.
575-
Looks like the render() function of "${hostTagName}" is returning an array that contains the <Host>.
575+
Looks like the render() function of "${hostTagName.toLowerCase()}" is returning an array that contains the <Host>.
576576
577577
The render() function should look like this instead:
578578

0 commit comments

Comments
 (0)