Merge pull request #3599 from owenconti/ft/3598-responses-shouldComponentUpdate

shouldComponentUpdate for response components

src/core/components/live-response.jsx

@@ -1,6 +1,7 @@
 import React from "react"
 import PropTypes from "prop-types"
 import ImPropTypes from "react-immutable-proptypes"
+import { Iterable } from "immutable"
 
 const Headers = ( { headers } )=>{
   return (
@@ -28,19 +29,29 @@ Duration.propTypes = {
 
 export default class LiveResponse extends React.Component {
   static propTypes = {
-    response: PropTypes.object.isRequired,
+    response: PropTypes.instanceOf(Iterable).isRequired,
-    specSelectors: PropTypes.object.isRequired,
+    path: PropTypes.string.isRequired,
-    pathMethod: PropTypes.object.isRequired,
+    method: PropTypes.string.isRequired,
-    getComponent: PropTypes.func.isRequired,
     displayRequestDuration: PropTypes.bool.isRequired,
+    specSelectors: PropTypes.object.isRequired,
+    getComponent: PropTypes.func.isRequired,
     getConfigs: PropTypes.func.isRequired
   }
 
+  shouldComponentUpdate(nextProps) {
+    // BUG: props.response is always coming back as a new Immutable instance
+    // same issue as responses.jsx (tryItOutResponse)
+    return this.props.response !== nextProps.response
+      || this.props.path !== nextProps.path
+      || this.props.method !== nextProps.method
+      || this.props.displayRequestDuration !== nextProps.displayRequestDuration
+  }
+
   render() {
-    const { response, getComponent, getConfigs, displayRequestDuration, specSelectors, pathMethod } = this.props
+    const { response, getComponent, getConfigs, displayRequestDuration, specSelectors, path, method } = this.props
     const { showMutatedRequest } = getConfigs()
 
-    const curlRequest = showMutatedRequest ? specSelectors.mutatedRequestFor(pathMethod[0], pathMethod[1]) : specSelectors.requestFor(pathMethod[0], pathMethod[1])
+    const curlRequest = showMutatedRequest ? specSelectors.mutatedRequestFor(path, method) : specSelectors.requestFor(path, method)
     const status = response.get("status")
     const url = response.get("url")
     const headers = response.get("headers").toJS()
@@ -111,7 +122,6 @@ export default class LiveResponse extends React.Component {
 
   static propTypes = {
     getComponent: PropTypes.func.isRequired,
-    request: ImPropTypes.map,
     response: ImPropTypes.map
   }
 }

src/core/components/operation.jsx

@@ -6,6 +6,8 @@ import { Iterable } from "immutable"
 export default class Operation extends PureComponent {
   static propTypes = {
     operation: PropTypes.instanceOf(Iterable).isRequired,
+    response: PropTypes.instanceOf(Iterable),
+    request: PropTypes.instanceOf(Iterable),
 
     toggleShown: PropTypes.func.isRequired,
     onTryoutClick: PropTypes.func.isRequired,
@@ -25,19 +27,21 @@ export default class Operation extends PureComponent {
   }
 
   static defaultProps = {
-    showSummary: true,
+    operation: null,
     response: null,
-    allowTryItOut: true,
+    request: null
-    displayOperationId: false,
-    displayRequestDuration: false
   }
 
   shouldComponentUpdate(nextProps) {
     return this.props.operation !== nextProps.operation
+      || this.props.response !== nextProps.response
+      || this.props.request !== nextProps.request
   }
 
   render() {
     let {
+      response,
+      request,
       toggleShown,
       onTryoutClick,
       onCancelClick,
@@ -69,8 +73,6 @@ export default class Operation extends PureComponent {
       tryItOutEnabled,
       executeInProgress
     } = operationProps.toJS()
-    let response = operationProps.get("response")
-    let request = operationProps.get("request")
 
     let {
       summary,
@@ -216,7 +218,8 @@ export default class Operation extends PureComponent {
                     specActions={ specActions }
                     produces={ produces }
                     producesValue={ operation.get("produces_value") }
-                    pathMethod={ [path, method] }
+                    path={ path }
+                    method={ method }
                     displayRequestDuration={ displayRequestDuration }
                     fn={fn} />
               }

src/core/components/response.jsx

@@ -1,7 +1,7 @@
 import React from "react"
 import PropTypes from "prop-types"
 import cx from "classnames"
-import { fromJS, Seq } from "immutable"
+import { fromJS, Seq, Iterable } from "immutable"
 import { getSampleSchema, fromJSOrdered } from "core/utils"
 
 const getExampleComponent = ( sampleResponse, examples, HighlightCode ) => {
@@ -42,7 +42,7 @@ export default class Response extends React.Component {
 
   static propTypes = {
     code: PropTypes.string.isRequired,
-    response: PropTypes.object,
+    response: PropTypes.instanceOf(Iterable),
     className: PropTypes.string,
     getComponent: PropTypes.func.isRequired,
     specSelectors: PropTypes.object.isRequired,
@@ -57,6 +57,12 @@ export default class Response extends React.Component {
     onContentTypeChange: () => {}
   };
 
+  shouldComponentUpdate(nextProps) {
+    return this.props.code !== nextProps.code
+      || this.props.response !== nextProps.response
+      || this.props.className !== nextProps.className
+  }
+
   _onContentTypeChange = (value) => {
     const { onContentTypeChange, controlsAcceptHeader } = this.props
     this.setState({ responseContentType: value })

src/core/components/responses.jsx

@@ -4,38 +4,49 @@ import { fromJS, Iterable } from "immutable"
 import { defaultStatusCode, getAcceptControllingResponse } from "core/utils"
 
 export default class Responses extends React.Component {
-
   static propTypes = {
-    request: PropTypes.instanceOf(Iterable),
     tryItOutResponse: PropTypes.instanceOf(Iterable),
     responses: PropTypes.instanceOf(Iterable).isRequired,
     produces: PropTypes.instanceOf(Iterable),
     producesValue: PropTypes.any,
+    displayRequestDuration: PropTypes.bool.isRequired,
+    path: PropTypes.string.isRequired,
+    method: PropTypes.string.isRequired,
     getComponent: PropTypes.func.isRequired,
     specSelectors: PropTypes.object.isRequired,
     specActions: PropTypes.object.isRequired,
     oas3Actions: PropTypes.object.isRequired,
-    pathMethod: PropTypes.array.isRequired,
-    displayRequestDuration: PropTypes.bool.isRequired,
     fn: PropTypes.object.isRequired,
     getConfigs: PropTypes.func.isRequired
   }
 
   static defaultProps = {
-    request: null,
     tryItOutResponse: null,
     produces: fromJS(["application/json"]),
     displayRequestDuration: false
   }
 
-  onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue(this.props.pathMethod, val)
+  shouldComponentUpdate(nextProps) {
+    // BUG: props.tryItOutResponse is always coming back as a new Immutable instance
+    let render = this.props.tryItOutResponse !== nextProps.tryItOutResponse 
+    || this.props.responses !== nextProps.responses
+    || this.props.produces !== nextProps.produces
+    || this.props.producesValue !== nextProps.producesValue
+    || this.props.displayRequestDuration !== nextProps.displayRequestDuration
+    || this.props.path !== nextProps.path
+    || this.props.method !== nextProps.method
+    return render
+  }
+
+	onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue(this.props.path, this.props.method, val)
 
   onResponseContentTypeChange = ({ controlsAcceptHeader, value }) => {
-    const { oas3Actions, pathMethod } = this.props
+    const { oas3Actions, path, method } = this.props
     if(controlsAcceptHeader) {
       oas3Actions.setResponseContentType({
         value,
-        pathMethod
+        path,
+        method
       })
     }
   }
@@ -43,7 +54,6 @@ export default class Responses extends React.Component {
   render() {
     let {
       responses,
-      request,
       tryItOutResponse,
       getComponent,
       getConfigs,
@@ -81,12 +91,12 @@ export default class Responses extends React.Component {
           {
             !tryItOutResponse ? null
                               : <div>
-                                  <LiveResponse request={ request }
+                                  <LiveResponse response={ tryItOutResponse }
-                                                response={ tryItOutResponse }
                                                 getComponent={ getComponent }
                                                 getConfigs={ getConfigs }
                                                 specSelectors={ specSelectors }
-                                                pathMethod={ this.props.pathMethod }
+                                                path={ this.props.path }
+                                                method={ this.props.method }
                                                 displayRequestDuration={ displayRequestDuration } />
                                   <h4>Responses</h4>
                                 </div>

src/core/containers/OperationContainer.jsx

@@ -98,7 +98,7 @@ export default class OperationContainer extends PureComponent {
   }
   
   shouldComponentUpdate(nextProps, nextState) {
-    return this.state.tryItOutEnabled !== nextState.tryItOutEnabled
+    const render = this.state.tryItOutEnabled !== nextState.tryItOutEnabled
       || this.state.executeInProgress !== nextState.executeInProgress
       || this.props.op !== nextProps.op
       || this.props.tag !== nextProps.tag
@@ -107,7 +107,6 @@ export default class OperationContainer extends PureComponent {
       || this.props.operationId !== nextProps.operationId
       || this.props.showSummary !== nextProps.showSummary
       || this.props.isShown !== nextProps.isShown
-      || this.props.isShownKey !== nextProps.isShownKey
       || this.props.jumpToKey !== nextProps.jumpToKey
       || this.props.allowTryItOut !== nextProps.allowTryItOut
       || this.props.displayOperationId !== nextProps.displayOperationId
@@ -115,6 +114,7 @@ export default class OperationContainer extends PureComponent {
       || this.props.response !== nextProps.response
       || this.props.request !== nextProps.request
       || this.props.isDeepLinkingEnabled !== nextProps.isDeepLinkingEnabled
+      return render
   }
 
   toggleShown =() => {
@@ -177,7 +177,6 @@ export default class OperationContainer extends PureComponent {
       isShownKey,
       jumpToKey,
       allowTryItOut,
-      response,
       request,
       displayOperationId,
       displayRequestDuration,
@@ -189,6 +188,8 @@ export default class OperationContainer extends PureComponent {
     return (
       <Operation
         operation={operationProps}
+        response={response}
+        request={request}
 
         toggleShown={this.toggleShown}
         onTryoutClick={this.onTryoutClick}

src/core/plugins/oas3/actions.js

@@ -28,10 +28,10 @@ export function setRequestContentType ({ value, pathMethod }) {
   }
 }
 
-export function setResponseContentType ({ value, pathMethod }) {
+export function setResponseContentType ({ value, path, method }) {
   return {
     type: UPDATE_RESPONSE_CONTENT_TYPE,
-    payload: { value, pathMethod }
+    payload: { value, path, method }
   }
 }
 

src/core/plugins/oas3/reducers.js

@@ -18,8 +18,7 @@ export default {
     let [path, method] = pathMethod
     return state.setIn( [ "requestData", path, method, "requestContentType" ], value)
   },
-  [UPDATE_RESPONSE_CONTENT_TYPE]: (state, { payload: { value, pathMethod } } ) =>{
+  [UPDATE_RESPONSE_CONTENT_TYPE]: (state, { payload: { value, path, method } } ) =>{
-    let [path, method] = pathMethod
     return state.setIn( [ "requestData", path, method, "responseContentType" ], value)
   },
   [UPDATE_SERVER_VARIABLE_VALUE]: (state, { payload: { server, key, val } } ) =>{

test/core/system/system.js

@@ -470,7 +470,7 @@ describe("bound system", function(){
     it("allows container components to provide their own `mapStateToProps` function", function() {
       // Given
       class ContainerComponent extends PureComponent {
-        static mapStateToProps(nextState, props) {
+        mapStateToProps(nextState, props) {
           return {
             "abc": "This came from mapStateToProps"
           }