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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cross browser assertions #1708

Merged
merged 7 commits into from
Jun 16, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compat/test/browser/portals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('Portal', () => {
let set;

function Foo(props) {
const [additionalProps, setProps] = useState({ style: { background: 'red' } });
const [additionalProps, setProps] = useState({ style: { backgroundColor: 'red' } });
set = (c) => setProps(c);
return (
<div>
Expand All @@ -94,11 +94,11 @@ describe('Portal', () => {
}

render(<Foo />, scratch);
expect(scratch.firstChild.style.background).to.equal('red');
expect(scratch.firstChild.style.backgroundColor).to.equal('red');

set({});
rerender();
expect(scratch.firstChild.style.background).to.equal('');
expect(scratch.firstChild.style.backgroundColor).to.equal('');
});

it('should not unmount the portal component', () => {
Expand Down
24 changes: 9 additions & 15 deletions debug/test/browser/debug.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,21 +171,15 @@ describe('debug', () => {
expect(fn).to.throw(/createElement/);
});

it('Should throw errors when accessing certain attributes', () => {
let Foo = () => <div />;
const oldOptionsVnode = options.vnode;
options.vnode = (vnode) => {
oldOptionsVnode(vnode);
expect(() => vnode).to.not.throw();
expect(() => vnode.attributes).to.throw(/use vnode.props/);
expect(() => vnode.nodeName).to.throw(/use vnode.type/);
expect(() => vnode.children).to.throw(/use vnode.props.children/);
expect(() => vnode.attributes = {}).to.throw(/use vnode.props/);
expect(() => vnode.nodeName = 'test').to.throw(/use vnode.type/);
expect(() => vnode.children = [<div />]).to.throw(/use vnode.props.children/);
};
render(<Foo />, scratch);
options.vnode = oldOptionsVnode;
it('should throw errors when accessing certain attributes', () => {
const vnode = h('div', null);
expect(() => vnode).to.not.throw();
expect(() => vnode.attributes).to.throw(/use vnode.props/);
expect(() => vnode.nodeName).to.throw(/use vnode.type/);
expect(() => vnode.children).to.throw(/use vnode.props.children/);
expect(() => vnode.attributes = {}).to.throw(/use vnode.props/);
expect(() => vnode.nodeName = 'test').to.throw(/use vnode.type/);
expect(() => vnode.children = [<div />]).to.throw(/use vnode.props.children/);
});

it('should print an error when component is an array', () => {
Expand Down
15 changes: 13 additions & 2 deletions src/diff/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { assign } from '../util';
*/
export function diffProps(dom, newProps, oldProps, isSvg) {
let i;

const keys = Object.keys(newProps).sort();
for (i = 0; i < keys.length; i++) {
const k = keys[i];
Expand Down Expand Up @@ -73,7 +73,18 @@ function setProperty(dom, name, value, oldValue, isSvg) {
}
}
else if (name!=='list' && name!=='tagName' && !isSvg && (name in dom)) {
dom[name] = value==null ? '' : value;
// Setting `select.value` doesn't work in IE11.
// Only `<select>` elements have the length property
if (dom.length && name=='value') {
for (name = dom.length; name--;) {
if (dom.options[name].value==value) {
dom.selectedIndex = name;
}
}
}
else {
dom[name] = value==null ? '' : value;
}
}
else if (typeof value!=='function' && name!=='dangerouslySetInnerHTML') {
if (name!==(name = name.replace(/^xlink:?/, ''))) {
Expand Down
20 changes: 13 additions & 7 deletions test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,17 @@ describe('render()', () => {
let div = scratch.childNodes[0];
expect(div.attributes.length).to.equal(2);

expect(div.attributes[0].name).equal('bar');
expect(div.attributes[0].value).equal('abc');
// Normalize attribute order because it's different in various browsers
let normalized = {};
for (let i = 0; i < div.attributes.length; i++) {
let attr = div.attributes[i];
normalized[attr.name] = attr.value;
}

expect(div.attributes[1].name).equal('foo');
expect(div.attributes[1].value).equal('[object Object]');
expect(normalized).to.deep.equal({
bar: 'abc',
foo: '[object Object]'
});
});

it('should apply class as String', () => {
Expand Down Expand Up @@ -396,9 +402,9 @@ describe('render()', () => {

it('should remove old styles', () => {
render(<div style={{ color: 'red' }} />, scratch);
render(<div style={{ background: 'blue' }} />, scratch);
expect(scratch.firstChild.style).to.have.property('color').that.equals('');
expect(scratch.firstChild.style).to.have.property('background').that.equals('blue');
render(<div style={{ backgroundColor: 'blue' }} />, scratch);
expect(scratch.firstChild.style.color).to.equal('');
expect(scratch.firstChild.style.backgroundColor).to.equal('blue');
});

// Skip test if the currently running browser doesn't support CSS Custom Properties
Expand Down