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

Throw when mount/shallow rendering invalid root elements #1759

Merged
merged 1 commit into from
Aug 18, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 42 additions & 0 deletions packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,48 @@ describeWithDOM('mount', () => {
expect(spy).to.have.property('callCount', 1);
});

describe('wrapping invalid elements', () => {
itIf(is('>= 16'), 'should throw when mounting Portals', () => {
const portal = createPortal(
<div />,
{ nodeType: 1 },
);

expect(() => mount(portal)).to.throw(
Error,
'ReactWrapper can only wrap valid elements',
);
});

it('should throw when mounting plain text', () => {
expect(() => mount('Foo')).to.throw(
Error,
'ReactWrapper can only wrap valid elements',
);
});

it('should throw when mounting multiple elements', () => {
expect(() => mount([<div />])).to.throw(
TypeError,
'ReactWrapper can only wrap valid elements',
);
});
});

it('should mount built in components', () => {
expect(() => mount(<div />)).not.to.throw();
});

it('should mount composite components', () => {
class Foo extends React.Component {
render() {
return <div />;
}
}

expect(() => mount(<Foo />)).not.to.throw();
});

describeIf(is('>= 16.3'), 'uses the isValidElementType from the Adapter to validate the prop type of Component', () => {
const Foo = () => null;
const Bar = () => null;
Expand Down
43 changes: 43 additions & 0 deletions packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import './_helpers/setupAdapters';
import {
createClass,
createContext,
createPortal,
createRef,
Fragment,
forwardRef,
Expand Down Expand Up @@ -77,6 +78,48 @@ describe('shallow', () => {
expect(wrapper.children().type()).to.equal('div');
expect(wrapper.children().props().bam).to.equal(undefined);
});

describe('wrapping invalid elements', () => {
itIf(is('>= 16'), 'should throw when shallow rendering Portals', () => {
const portal = createPortal(
<div />,
{ nodeType: 1 },
);

expect(() => shallow(portal)).to.throw(
Error,
'ShallowWrapper can only wrap valid elements',
);
});

it('should throw when shallow rendering plain text', () => {
expect(() => shallow('Foo')).to.throw(
Error,
'ShallowWrapper can only wrap valid elements',
);
});

it('should throw when shallow rendering multiple elements', () => {
expect(() => shallow([<div />])).to.throw(
TypeError,
'ShallowWrapper can only wrap valid elements',
);
});
});

it('should shallow render built in components', () => {
expect(() => shallow(<div />)).not.to.throw();
});

it('should shallow render composite components', () => {
class Foo extends React.Component {
render() {
return <div />;
}
}

expect(() => shallow(<Foo />)).not.to.throw();
});
});

describe('context', () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/enzyme/src/ReactWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,13 @@ class ReactWrapper {
const options = makeOptions(passedOptions);

if (!root) {
const adapter = getAdapter(options);
if (!adapter.isValidElement(nodes)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, nodes could be an array of valid elements; i'm not sure we can check it directly.

also, i think we probably want to check this on root too if that's provided?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we try to mount an array of elements as the root that should still throw.

root elements should always have already been through this path when they were created, ie wrap/ReactWrapper constructor only get called with null when mounting (or diving in the shallow wrapper) or this[ROOT] when traversing, which is only set here.

throw new TypeError('ReactWrapper can only wrap valid elements');
}

privateSet(this, UNRENDERED, nodes);
const renderer = getAdapter(options).createRenderer({ mode: 'mount', ...options });
const renderer = adapter.createRenderer({ mode: 'mount', ...options });
privateSet(this, RENDERER, renderer);
renderer.render(nodes, options.context);
privateSet(this, ROOT, this);
Expand Down
4 changes: 4 additions & 0 deletions packages/enzyme/src/ShallowWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ class ShallowWrapper {

// mounting a ShallowRender component
if (!root) {
if (!adapter.isValidElement(nodes)) {
throw new TypeError('ShallowWrapper can only wrap valid elements');
}

privateSet(this, ROOT, this);
privateSet(this, UNRENDERED, nodes);
const renderer = adapter.createRenderer({ mode: 'shallow', ...options });
Expand Down