more PR feedback

aweary · Aug 15, 2017 · 6966eda · 6966eda
1 parent bd93716
commit 6966eda
Show file tree

Hide file tree

Showing 16 changed files with 66 additions and 119 deletions.
diff --git a/docs/future/migration.md b/docs/future/migration.md
@@ -94,23 +94,29 @@ two elements being found.
 Although this is a breaking change, I believe the new behavior is closer to what people would
 actually expect and want.
 
-## `get(n)` versus `getElement(n)` versus `getNode()`
-
-NOTE: might be able to get rid of this
-
-get(n) v2: returns the react element that the wrapper wraps at index n
-get(n) v3: returns the RST node that the wrapper wraps at index n
-
-getNode() v2: returns the react element that the wrapper wraps (must be single)
-getNode() v3: returns the RST node that the wrapper wraps (must be single)
-
-getElement(n) v3: effectively what `get(n)` was in v2
-
+## `children()` now has slightly different meaning
 
+Enzyme has a `.children()` method which is intended to return the rendered children of a wrapper.
 
-## `children()` now has slightly different meaning
+When using `mount(...)`, it can sometimes be unclear exactly what this would mean. Consider for
+example the following react components:
 
-TODO: talk about this
+```js
+class Box extends React.Component {
+  render() {
+    return <div className="box">{this.props.children}</div>;
+  }
+}
+class Foo extends React.Component {
+  render() {
+    return (
+      <Box bam>
+        <div className="div" />
+      </Box>
+    );
+  }
+}
+```
 
 ## For `mount`, updates are sometimes required when they weren't before
 
@@ -358,12 +364,4 @@ const wrapper = mount(<Foo outer={x} />);
 expect(wrapper.props()).to.deep.equal({ outer: x });
 ```
 
-## for shallow, getNode() was renamed to getElement()
-
 ## for mount, getNode() should not be used. instance() does what it used to.
-
-## for mount, getElement() will return the root JSX element
-
-## what getNode() returns
-
-we need to keep in mind that `getElement()` will no longer be referentially equal to what it was before.
diff --git a/karma.conf.js b/karma.conf.js
@@ -3,7 +3,7 @@
 require('babel-register');
 
 var IgnorePlugin = require('webpack').IgnorePlugin;
-var is = require('./src/version').is;
+var is = require('./test/version').is;
 
 function getPlugins() {
   const adapter13 = new IgnorePlugin(/adapters\/ReactThirteenAdapter/);

diff --git a/package.json b/package.json
@@ -108,7 +108,8 @@
     "rimraf": "^2.6.1",
     "safe-publish-latest": "^1.1.1",
     "sinon": "^2.4.1",
-    "webpack": "^1.13.3"
+    "webpack": "^1.13.3",
+    "react": "0.13.x || 0.14.x || ^15.0.0-0 || ^16.0.0-0"
   },
   "peerDependencies": {
     "react": "0.13.x || 0.14.x || ^15.0.0-0 || ^16.0.0-0"

diff --git a/setupAdapters.js b/setupAdapters.js
diff --git a/src/ReactWrapper.jsx b/src/ReactWrapper.jsx
@@ -116,10 +116,6 @@ class ReactWrapper {
     );
   }
 
-  rendered() {
-    return this.single('rendered', n => this.wrap(n.rendered));
-  }
-
   /**
    * Returns the wrapped component.
    *

diff --git a/src/adapters/ReactThirteenAdapter.js b/src/adapters/ReactThirteenAdapter.js
@@ -5,8 +5,8 @@ import PropTypes from 'prop-types';
 import values from 'object.values';
 import EnzymeAdapter from './EnzymeAdapter';
 import elementToTree from './elementToTree';
+import mapNativeEventNames from './ReactThirteenMapNativeEventNames'
 import {
-  mapNativeEventNames,
   propFromEvent,
   withSetStateAllowed,
   assertDomAvailable,

diff --git a/src/adapters/Utils.js b/src/adapters/Utils.js
@@ -1,5 +1,3 @@
-import { REACT013 } from '../version';
-
 export function mapNativeEventNames(event) {
   const nativeToReactEventMap = {
     compositionend: 'compositionEnd',
@@ -36,15 +34,10 @@ export function mapNativeEventNames(event) {
     timeupdate: 'timeUpdate',
     volumechange: 'volumeChange',
     beforeinput: 'beforeInput',
+    mouseenter: 'mouseEnter',
+    mouseleave: 'mouseLeave',
   };
 
-  if (!REACT013) {
-    // these could not be simulated in React 0.13:
-    // https://github.com/facebook/react/issues/1297
-    nativeToReactEventMap.mouseenter = 'mouseEnter';
-    nativeToReactEventMap.mouseleave = 'mouseLeave';
-  }
-
   return nativeToReactEventMap[event] || event;
 }
 

diff --git a/src/version.js b/src/version.js
diff --git a/test/Adapter-spec.jsx b/test/Adapter-spec.jsx
@@ -1,8 +1,8 @@
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import { expect } from 'chai';
 
-import { REACT013, REACT16 } from '../src/version';
+import { REACT013, REACT16 } from './version';
 import configuration from '../src/configuration';
 import { itIf, describeWithDOM } from './_helpers';
 

diff --git a/test/ComplexSelector-spec.jsx b/test/ComplexSelector-spec.jsx
@@ -1,4 +1,4 @@
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import { expect } from 'chai';
 

diff --git a/test/Debug-spec.jsx b/test/Debug-spec.jsx
@@ -1,4 +1,4 @@
-import '../setupAdapters';
+import './setupAdapters';
 import { expect } from 'chai';
 import React from 'react';
 import {
@@ -13,7 +13,7 @@ import {
   describeIf,
   itIf,
 } from './_helpers';
-import { REACT013 } from '../src/version';
+import { REACT013 } from './version';
 import configuration from '../src/configuration';
 
 const { adapter } = configuration.get();

diff --git a/test/RSTTraversal-spec.jsx b/test/RSTTraversal-spec.jsx
@@ -1,4 +1,4 @@
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import sinon from 'sinon';
 import { expect } from 'chai';
@@ -16,7 +16,7 @@ import {
   buildPredicate,
 } from '../src/RSTTraversal';
 import { describeIf } from './_helpers';
-import { REACT013 } from '../src/version';
+import { REACT013 } from './version';
 
 const $ = elementToTree;
 

diff --git a/test/ReactWrapper-spec.jsx b/test/ReactWrapper-spec.jsx
@@ -1,5 +1,5 @@
 /* globals document */
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { expect } from 'chai';
@@ -1916,10 +1916,10 @@ describeWithDOM('mount', () => {
           ]}
         />,
       );
-      expect(wrapper.rendered().children().length).to.equal(3);
-      expect(wrapper.rendered().children().at(0).hasClass('foo')).to.equal(true);
-      expect(wrapper.rendered().children().at(1).hasClass('bar')).to.equal(true);
-      expect(wrapper.rendered().children().at(2).hasClass('baz')).to.equal(true);
+      expect(wrapper.children().children().length).to.equal(3);
+      expect(wrapper.children().children().at(0).hasClass('foo')).to.equal(true);
+      expect(wrapper.children().children().at(1).hasClass('bar')).to.equal(true);
+      expect(wrapper.children().children().at(2).hasClass('baz')).to.equal(true);
     });
 
     it('should optionally allow a selector to filter by', () => {
@@ -1959,10 +1959,10 @@ describeWithDOM('mount', () => {
             ]}
           />,
         );
-        expect(wrapper.rendered().children().length).to.equal(3);
-        expect(wrapper.rendered().children().at(0).hasClass('foo')).to.equal(true);
-        expect(wrapper.rendered().children().at(1).hasClass('bar')).to.equal(true);
-        expect(wrapper.rendered().children().at(2).hasClass('baz')).to.equal(true);
+        expect(wrapper.children().children().length).to.equal(3);
+        expect(wrapper.children().children().at(0).hasClass('foo')).to.equal(true);
+        expect(wrapper.children().children().at(1).hasClass('bar')).to.equal(true);
+        expect(wrapper.children().children().at(2).hasClass('baz')).to.equal(true);
       });
     });
   });
@@ -2153,12 +2153,12 @@ describeWithDOM('mount', () => {
         expect(wrapper.hasClass('FoOo')).to.equal(false);
         expect(wrapper.hasClass('doesnt-exist')).to.equal(false);
 
-        expect(wrapper.rendered().hasClass('foo')).to.equal(true);
-        expect(wrapper.rendered().hasClass('bar')).to.equal(true);
-        expect(wrapper.rendered().hasClass('baz')).to.equal(true);
-        expect(wrapper.rendered().hasClass('some-long-string')).to.equal(true);
-        expect(wrapper.rendered().hasClass('FoOo')).to.equal(true);
-        expect(wrapper.rendered().hasClass('doesnt-exist')).to.equal(false);
+        expect(wrapper.children().hasClass('foo')).to.equal(true);
+        expect(wrapper.children().hasClass('bar')).to.equal(true);
+        expect(wrapper.children().hasClass('baz')).to.equal(true);
+        expect(wrapper.children().hasClass('some-long-string')).to.equal(true);
+        expect(wrapper.children().hasClass('FoOo')).to.equal(true);
+        expect(wrapper.children().hasClass('doesnt-exist')).to.equal(false);
       });
     });
 
@@ -2178,12 +2178,12 @@ describeWithDOM('mount', () => {
         expect(wrapper.hasClass('FoOo')).to.equal(false);
         expect(wrapper.hasClass('doesnt-exist')).to.equal(false);
 
-        expect(wrapper.rendered().hasClass('foo')).to.equal(true);
-        expect(wrapper.rendered().hasClass('bar')).to.equal(true);
-        expect(wrapper.rendered().hasClass('baz')).to.equal(true);
-        expect(wrapper.rendered().hasClass('some-long-string')).to.equal(true);
-        expect(wrapper.rendered().hasClass('FoOo')).to.equal(true);
-        expect(wrapper.rendered().hasClass('doesnt-exist')).to.equal(false);
+        expect(wrapper.children().hasClass('foo')).to.equal(true);
+        expect(wrapper.children().hasClass('bar')).to.equal(true);
+        expect(wrapper.children().hasClass('baz')).to.equal(true);
+        expect(wrapper.children().hasClass('some-long-string')).to.equal(true);
+        expect(wrapper.children().hasClass('FoOo')).to.equal(true);
+        expect(wrapper.children().hasClass('doesnt-exist')).to.equal(false);
       });
     });
 
@@ -2209,13 +2209,13 @@ describeWithDOM('mount', () => {
         expect(wrapper.hasClass('doesnt-exist')).to.equal(false);
 
         // NOTE(lmr): the fact that this no longer works is a semantically
-        // meaningfull deviation in behavior
-        expect(wrapper.rendered().hasClass('foo')).to.equal(false);
-        expect(wrapper.rendered().hasClass('bar')).to.equal(false);
-        expect(wrapper.rendered().hasClass('baz')).to.equal(false);
-        expect(wrapper.rendered().hasClass('some-long-string')).to.equal(false);
-        expect(wrapper.rendered().hasClass('FoOo')).to.equal(false);
-        expect(wrapper.rendered().hasClass('doesnt-exist')).to.equal(false);
+        // meaningfull deviation in behavior. But this will be remedied with the ".root()" change
+        expect(wrapper.children().hasClass('foo')).to.equal(false);
+        expect(wrapper.children().hasClass('bar')).to.equal(false);
+        expect(wrapper.children().hasClass('baz')).to.equal(false);
+        expect(wrapper.children().hasClass('some-long-string')).to.equal(false);
+        expect(wrapper.children().hasClass('FoOo')).to.equal(false);
+        expect(wrapper.children().hasClass('doesnt-exist')).to.equal(false);
       });
     });
 

diff --git a/test/Utils-spec.jsx b/test/Utils-spec.jsx
@@ -1,4 +1,4 @@
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import { expect } from 'chai';
 
@@ -17,7 +17,7 @@ import {
   mapNativeEventNames,
   propFromEvent,
 } from '../src/adapters/Utils';
-import { REACT013 } from '../src/version';
+import { REACT013 } from './version';
 
 describe('Utils', () => {
   describe('nodeEqual', () => {

diff --git a/test/_helpers/react-compat.js b/test/_helpers/react-compat.js
@@ -4,7 +4,7 @@
   import/prefer-default-export: 0,
 */
 
-import { is } from '../../src/version';
+import { is } from '../version';
 
 let createClass;
 

diff --git a/test/staticRender-spec.jsx b/test/staticRender-spec.jsx
@@ -1,10 +1,10 @@
-import '../setupAdapters';
+import './setupAdapters';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { expect } from 'chai';
 import { describeWithDOM, describeIf } from './_helpers';
 import { render } from '../src/';
-import { REACT013 } from '../src/version';
+import { REACT013 } from './version';
 import { createClass } from './_helpers/react-compat';
 
 describeWithDOM('render', () => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -116,10 +116,6 @@ class ReactWrapper { @@
         );
       }
-      rendered() {
-        return this.single('rendered', n => this.wrap(n.rendered));
-      }
       /**
        * Returns the wrapped component.
        *
@@ Expand Down @@