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

Enzyme 3 #198

Merged
merged 13 commits into from
Oct 3, 2017
17 changes: 9 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
},
"peerDependencies": {
"chai": "^3.0.0 || ^4.0.0",
"cheerio": "0.19.x || 0.20.x || 0.22.x || 1.0.0-rc.1",
"enzyme": "1.x || ^2.3.0",
"react": "^0.14.0 || ^15.0.0-0",
"react-dom": "^0.14.0 || ^15.0.0-0"
"cheerio": "0.19.x || 0.20.x || 0.22.x || 1.0.0-rc.2",
Copy link
Member

Choose a reason for hiding this comment

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

enzyme 3 requires cheerio v1 or higher - this needs to change to ^1.0.0-rc.2

Copy link
Contributor

Choose a reason for hiding this comment

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

I think #196 has got the dependencies pretty much right

Copy link
Member

Choose a reason for hiding this comment

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

yes as a nonbreaking change #196 does this much better (altho enzyme 2 requires cheerio 0.22 so the 0.19 and 0.20 peer deps aren't helpful)

"enzyme": "^3.0.0",
"react": "^0.14.0 || ^15.0.0-0 || ^16.0.0",
"react-dom": "^0.14.0 || ^15.0.0-0 || ^16.0.0"
},
"keywords": [
"javascript",
Expand Down Expand Up @@ -58,13 +58,14 @@
"babel-register": "^6.5.2",
"chai": "^3.0.0 || ^4.0.0",
"cross-env": "3.1.4",
"enzyme": "^2.3.0",
"enzyme": "3.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

nothing should ever be pinned; these should all use ^.

"enzyme-adapter-react-16": "1.0.0",
"jsdom": "^9.1.0",
"mocha": "^3.0.0",
"prop-types": "^15.5.7",
"react": "^0.14.0 || ^15.0.0-0",
"react-addons-test-utils": "^0.14.0 || ^15.0.0-0",
"react-dom": "^0.14.0 || ^15.0.0-0",
"react": "16.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

dev dep ranges should always exactly match peer dep ranges.

"react-dom": "16.0.0",
"react-test-renderer": "16.0.0",
"rimraf": "^2.5.0",
"snazzy": "^5.0.0",
"standard": "^8.1.0"
Expand Down
5 changes: 2 additions & 3 deletions src/CheerioTestWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ export default class CheerioTestWrapper extends TestWrapper {

get el () {
if (!this.__el) {
if (this.wrapper.first()['0'].type === 'root') {
this.__el = this.wrapper.first()
if (this.__el.length > 0 && this.__el['0'].type === 'root') {
this.__el = this.wrapper.children().first()
} else {
this.__el = this.wrapper.first()
}
}

Expand Down
11 changes: 4 additions & 7 deletions src/ReactTestWrapper.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import {findDOMNode} from 'enzyme/build/react-compat'

import TestWrapper from './TestWrapper'

export default class ReactTestWrapper extends TestWrapper {
Expand All @@ -10,18 +8,17 @@ export default class ReactTestWrapper extends TestWrapper {

get el () {
if (!this.__el) {
this.__el = this.wrapper.single((n) => findDOMNode(n))
this.__el = this.wrapper.getDOMNode()
}

return this.__el
}

inspect () {
const name = this.wrapper.root.node.constructor.displayName ||
this.wrapper.root.node.constructor.name ||
'???'
const root = this.wrapper.root()
const name = root.name() || '???'

if (this.wrapper.root === this.wrapper) {
if (root === this.wrapper) {
return `<${name} />`
}

Expand Down
12 changes: 8 additions & 4 deletions src/ShallowTestWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import $ from 'cheerio'

import TestWrapper from './TestWrapper'

const getDisplayName = type => type.displayName || type.name || type
Copy link
Member

Choose a reason for hiding this comment

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

why is this function needed?


export default class ShallowTestWrapper extends TestWrapper {
constructor (wrapper) {
super()
Expand All @@ -17,11 +19,13 @@ export default class ShallowTestWrapper extends TestWrapper {
}

inspect () {
const name = this.wrapper.root.unrendered.type.displayName ||
this.wrapper.root.unrendered.type.name ||
'???'
const root = this.wrapper.root()

const rootInstance = root.instance()
const rootType = rootInstance ? rootInstance.constructor : root.getElement().type
const name = rootType ? getDisplayName(rootType) : '???'
Copy link
Member

Choose a reason for hiding this comment

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

root.name() || '???' should suffice here, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enzyme's ShallowWrapper behaves slightly different from the ReactWrapper when it comes to the name() function. Given the following component:

const Example = () => <table />;

mount(<Example />).name() returns 'Example' while shallow(<Example />).name() returns 'table'.


if (this.wrapper.root === this.wrapper) {
if (root === this.wrapper) {
return `<${name} />`
}

Expand Down
2 changes: 1 addition & 1 deletion src/wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default function wrap (el) {
return new ReactTestWrapper(el)
}

if (el && el._root && el.options) {
if (el && el.cheerio && el.options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need a fallback (to _root for old cheerio versions)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks Cheerio.prototype.cheerio has been around since 0.19.0: https://github.com/cheeriojs/cheerio/blob/0.19.0/lib/cheerio.js#L99

return new CheerioTestWrapper(el)
}
}
14 changes: 9 additions & 5 deletions test/attr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@ class Fixture extends React.Component {
<div id='root'>
<span id='child'>test</span>
<video itemScope allowFullScreen={true} autoPlay={''} hidden={false} controls={null} loop={undefined}>test2</video>
<audio role name={''} accessKey={false} spellCheck={null} rel={magicToStringObject}>test3</audio>
<tr rowSpan={0} rows={0} cols={'4'} size={'0'} />
<audio disabled name={''} contentEditable={false} spellCheck={null} rel={magicToStringObject}>test3</audio>
<table>
<tbody>
<tr rowSpan={0} rows={0} cols={'4'} size={'0'} />
</tbody>
</table>
</div>
)
/* eslint-enable react/jsx-boolean-value */
Expand Down Expand Up @@ -86,15 +90,15 @@ describe('#attr', () => {

describe('regular attrs', () => {
it('passes when attribute exists without a value', (wrapper) => {
expect(wrapper.find('audio')).to.have.attr('role')
expect(wrapper.find('audio')).to.have.attr('disabled')
})

it('passes when attribute exists with a falsey (but not false/null/undefined) value', (wrapper) => {
expect(wrapper.find('audio')).to.have.attr('name')
})

it('passes when attribute exists but has value `false`', (wrapper) => {
expect(wrapper.find('audio')).to.have.attr('accesskey')
expect(wrapper.find('audio')).to.have.attr('contenteditable')
})

it('passes negated when attribute exists but has value `null`', (wrapper) => {
Expand Down Expand Up @@ -148,7 +152,7 @@ describe('#attr', () => {

describe('regular attrs', () => {
it('converts values to strings', (wrapper) => {
expect(wrapper.find('audio')).to.have.attr('accesskey', 'false')
expect(wrapper.find('audio')).to.have.attr('contenteditable', 'false')
expect(wrapper.find('audio')).to.have.attr('rel', 'magic')
})
})
Expand Down
6 changes: 4 additions & 2 deletions test/blank.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
class Fixture extends React.Component {
render () {
return (
<div id='parent'>
<div id='child' />
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are all these necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enzyme 3 changes the way it wraps HTML when returning from the render function. The new version returns a cheerio wrapper with a type of tag that IS the parent element (instead of a wrapper with a type root that contains the parent element).

For example:

<div id='parent'>
  <div id='child' />
<div>

With the above code in Enzyme 3, calling wrapper.find('#parent').length returns 0 while calling wrapper.is('#parent') returns true. With Enzyme 2, wrapper.find('#parent').length returns 1 and wrapper.is('#parent') returns false.

Copy link
Member

Choose a reason for hiding this comment

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

(this is as a result of a breaking change in cheerio v1)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for explaining, we'll definitely have to document this in our CHANGELOG.md to educate people on how to upgrade

Copy link
Contributor

Choose a reason for hiding this comment

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

Crazy 3AM idea (jetlag), can we add the extra <div /> in chai-enzyme when using render?

Copy link
Member

Choose a reason for hiding this comment

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

You could wrap it, sure, but then anyone who didn't understand this, and was checking any other method besides .html(), would have the wrong assertions.

In other words, it's better for wrapper.is('#parent') === true and wrapper.html() not to contain the parent div, then for the reverse to be true.

<div id='parent'>
<div id='child' />
</div>
</div>
)
}
Expand Down
10 changes: 6 additions & 4 deletions test/descendants.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
class Fixture extends React.Component {
render () {
return (
<div id='root'>
<span id='child'>
<span id='last' />
</span>
<div>
<div id='root'>
<span id='child'>
<span id='last' />
</span>
</div>
</div>
)
}
Expand Down
6 changes: 4 additions & 2 deletions test/empty.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
class Fixture extends React.Component {
render () {
return (
<div id='parent'>
<div id='child' />
<div>
<div id='parent'>
<div id='child' />
</div>
</div>
)
}
Expand Down
24 changes: 13 additions & 11 deletions test/exactly.test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
class Fixture extends React.Component {
render () {
return (
<div id='root'>
<div id='child'>
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<span id='last' />
<div>
<div id='root'>
<div id='child'>
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<div className='multiple' />
<span id='last' />
</div>
</div>
</div>
)
Expand Down
4 changes: 3 additions & 1 deletion test/exist.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class Fixture extends React.Component {
render () {
return (
<div id='parent' />
<div>
<div id='parent' />
</div>
)
}
}
Expand Down
11 changes: 7 additions & 4 deletions test/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ class ClassSyntax extends React.Component {
}
}

const DisplayNameSyntax = React.createClass({
displayName: 'DisplayNameSyntax',

const DisplayNameSyntax = class extends React.Component {
render () {
return (
<div>Hello world</div>
)
}
})
}

DisplayNameSyntax.displayName = 'DisplayNameSyntax'

function inspect (wrapper) {
return wrap(wrapper).inspect()
Expand All @@ -29,6 +29,8 @@ describe('#inspect', () => {
expect(String(inspect(shallow(<ClassSyntax />)))).to.equal('<ClassSyntax />')
expect(String(inspect(shallow(<DisplayNameSyntax />)))).to.equal('<DisplayNameSyntax />')
expect(String(inspect(shallow(<DisplayNameSyntax />).find('div')))).to.equal('the node in <DisplayNameSyntax />')
expect(String(inspect(shallow(<DisplayNameSyntax />).find('span')))).to.equal('the node in <DisplayNameSyntax />')
expect(String(inspect(shallow(<div />)))).to.equal('<div />')
})
})

Expand All @@ -37,6 +39,7 @@ describe('#inspect', () => {
expect(String(inspect(mount(<ClassSyntax />)))).to.equal('<ClassSyntax />')
expect(String(inspect(mount(<DisplayNameSyntax />)))).to.equal('<DisplayNameSyntax />')
expect(String(inspect(mount(<DisplayNameSyntax />).find('div')))).to.equal('the node in <DisplayNameSyntax />')
expect(String(inspect(mount(<DisplayNameSyntax />).find('span')))).to.equal('the node in <DisplayNameSyntax />')
})
})

Expand Down
8 changes: 5 additions & 3 deletions test/match.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ class MyComponent extends React.Component {
class Fixture extends React.Component {
render () {
return (
<div id='root'>
<span id='child'>test</span>
<MyComponent id='my-component' />
<div>
<div id='root'>
<span id='child'>test</span>
<MyComponent id='my-component' />
</div>
</div>
)
}
Expand Down
14 changes: 12 additions & 2 deletions test/null.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,22 @@ describe('#present', () => {
describe('()', () => {
it('passes when the actual matches the expected', (wrapper) => {
expect(wrapper).to.be.present()
})
}, { render: false })

it('passes when the actual matches the expected', (wrapper) => {
expect(wrapper).not.to.be.present()
}, { shallow: false, mount: false })

it('fails when the actual does not match the expected', (wrapper) => {
expect(() => {
expect(wrapper).to.not.be.present()
}).to.throw('not to exist')
})
}, { render: false })

it('fails when the actual does not match the expected', (wrapper) => {
expect(() => {
expect(wrapper).to.be.present()
}).to.throw('to exist')
}, { shallow: false, mount: false })
})
})
4 changes: 3 additions & 1 deletion test/present.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class Fixture extends React.Component {
render () {
return (
<div id='parent' />
<div>
<div id='parent' />
</div>
)
}
}
Expand Down
4 changes: 4 additions & 0 deletions test/support/helper.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import jsdom from 'jsdom'
import chai from 'chai'
import Enzyme from 'enzyme'
import Adapter from 'enzyme-adapter-react-16'

import plugin from '../../src'

Enzyme.configure({ adapter: new Adapter() })

const doc = jsdom.jsdom('<!doctype html><html><body></body></html>')
const win = doc.defaultView

Expand Down
Loading