Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined can clear checked and value properties for custom elements #3813

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/diff/index.js
Expand Up @@ -332,6 +332,8 @@ function diffElementNodes(
let oldProps = oldVNode.props;
let newProps = newVNode.props;
let nodeType = newVNode.type;
let isCustomElement = ('' + newVNode.type).indexOf('-') > -1;

let i = 0;

// Tracks entering and exiting SVG namespace when descending through the tree.
Expand Down Expand Up @@ -420,7 +422,7 @@ function diffElementNodes(
}
}

diffProps(dom, newProps, oldProps, isSvg, isHydrating);
diffProps(dom, newProps, oldProps, isSvg, isHydrating, isCustomElement);

// If the new vnode didn't have dangerouslySetInnerHTML, diff its children
if (newHtml) {
Expand Down
13 changes: 10 additions & 3 deletions src/diff/props.js
Expand Up @@ -9,8 +9,16 @@ import options from '../options';
* @param {object} oldProps The old props
* @param {boolean} isSvg Whether or not this node is an SVG node
* @param {boolean} hydrate Whether or not we are in hydration mode
* @param {boolean} isCustomElement Whether or not we are diffing a custom element.
*/
export function diffProps(dom, newProps, oldProps, isSvg, hydrate) {
export function diffProps(
dom,
newProps,
oldProps,
isSvg,
hydrate,
isCustomElement
) {
let i;

for (i in oldProps) {
Expand All @@ -24,8 +32,7 @@ export function diffProps(dom, newProps, oldProps, isSvg, hydrate) {
(!hydrate || typeof newProps[i] == 'function') &&
i !== 'children' &&
i !== 'key' &&
i !== 'value' &&
i !== 'checked' &&
(isCustomElement || (i !== 'value' && i !== 'checked')) &&
oldProps[i] !== newProps[i]
) {
setProperty(dom, i, newProps[i], oldProps[i], isSvg);
Expand Down
53 changes: 53 additions & 0 deletions test/browser/render.test.js
Expand Up @@ -489,6 +489,59 @@ describe('render()', () => {
expect(scratch.innerHTML).to.equal('<o-input value="test"></o-input>');
});

describe('Custom elements properties', () => {
// Properties set to null/undefined are cleared in the DOM through an empty string.
const clearedPropertyValue = '';

class TestComponent extends HTMLElement {
constructor() {
super();
this.value = {};
this.checked = undefined;
}
}
customElements.define('test-component', TestComponent);

it(`should set complex value property`, () => {
const value = { foo: 'bar' };
render(<test-component value={value} />, scratch);

expect(scratch.querySelector('test-component').value).to.equal(value);
});

it(`should set checked property`, () => {
let initialValue = true;
render(<test-component checked={initialValue} />, scratch);
expect(scratch.querySelector('test-component').checked).to.equal(true);
});

clearPropertyWith(null);
clearPropertyWith(undefined);

function clearPropertyWith(clearValue) {
it(`should clear existing value property with ${clearValue}`, () => {
render(<test-component value={{ foo: 'bar' }} />, scratch);

render(<test-component value={clearValue} />, scratch);

expect(scratch.querySelector('test-component').value).to.equal(
clearedPropertyValue
);
});

it(`should clear existing checked property with ${clearValue}`, () => {
let initialValue = true;
render(<test-component checked={initialValue} />, scratch);

render(<test-component checked={clearValue} />, scratch);

expect(scratch.querySelector('test-component').checked).to.equal(
clearedPropertyValue
);
});
}
});

it('should mask value on password input elements', () => {
render(<input value="xyz" type="password" />, scratch);
expect(scratch.innerHTML).to.equal('<input type="password">');
Expand Down