Skip to content

Commit

Permalink
Remove nullish coalescing for textContent.
Browse files Browse the repository at this point in the history
The default browser behavior when setting `.textContent` to `null` or
`undefined` is to ultimately set the content to `''`†. This is quite
different from the behavior of `createTextNode` though… by splitting
the text node creation into multiple steps we can consistently leverage
`.textContent` and remove the need to `?? ''`.

† Note that the WHATWG spec only defines that behavior for `null`, but
  in practice all modern browsers seem to treat `undefined` similarly.
  • Loading branch information
theengineear committed Nov 12, 2024
1 parent b4b0c97 commit 6df5f71
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
13 changes: 12 additions & 1 deletion test/test-basic-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ class TestElement extends XElement {
type: String,
default: null,
},
undefinedProperty: {
type: String,
},
typelessProperty: {},
typelessPropertyWithCustomAttribute: {
attribute: 'custom-attribute-typeless',
Expand All @@ -33,12 +36,13 @@ class TestElement extends XElement {
};
}
static template(html) {
return ({ normalProperty, camelCaseProperty, numericProperty, nullProperty }) => {
return ({ normalProperty, camelCaseProperty, numericProperty, nullProperty, undefinedProperty }) => {
return html`
<div id="normal">${normalProperty}</div>
<span id="camel">${camelCaseProperty}</span>
<span id="num">${numericProperty}</span>
<span id="nul">${nullProperty}</span>
<span id="undef">${undefinedProperty}</span>
`;
};
}
Expand All @@ -60,6 +64,12 @@ it('renders an empty string in place of null value', () => {
assert(el.shadowRoot.getElementById('nul').textContent === '');
});

it('renders an empty string in place of undefined value', () => {
const el = document.createElement('test-element');
document.body.append(el);
assert(el.shadowRoot.getElementById('undef').textContent === '');
});

it('property setter updates on next micro tick after connect', async () => {
const el = document.createElement('test-element');
el.camelCaseProperty = 'Nonconforming';
Expand Down Expand Up @@ -99,6 +109,7 @@ it('observes all dash-cased versions of public, typeless, serializable, and decl
'camel-case-property',
'numeric-property',
'null-property',
'undefined-property',
'typeless-property',
'custom-attribute-typeless',
];
Expand Down
5 changes: 3 additions & 2 deletions x-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -1488,10 +1488,11 @@ class TemplateEngine {
}
const previousSibling = node.previousSibling;
if (previousSibling === startNode) {
const textNode = document.createTextNode(value ?? '');
const textNode = document.createTextNode('');
textNode.textContent = value;
node.parentNode.insertBefore(textNode, node);
} else {
previousSibling.textContent = value ?? '';
previousSibling.textContent = value;
}
}
}
Expand Down

0 comments on commit 6df5f71

Please sign in to comment.