From bcefb6eb3e00d857ab122d8c77ff8537286cbe0b Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 20 Aug 2017 15:25:13 -0600 Subject: [PATCH 1/4] Convert `pathMethod` prop to two separate props. --- src/core/components/live-response.jsx | 7 ++++--- src/core/components/operation.jsx | 3 ++- src/core/components/responses.jsx | 28 ++++++++++++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/core/components/live-response.jsx b/src/core/components/live-response.jsx index e9e3fa2d..457e3306 100644 --- a/src/core/components/live-response.jsx +++ b/src/core/components/live-response.jsx @@ -30,17 +30,18 @@ export default class LiveResponse extends React.Component { static propTypes = { response: PropTypes.object.isRequired, specSelectors: PropTypes.object.isRequired, - pathMethod: PropTypes.object.isRequired, + path: PropTypes.string.isRequired, + method: PropTypes.string.isRequired, getComponent: PropTypes.func.isRequired, displayRequestDuration: PropTypes.bool.isRequired, getConfigs: PropTypes.func.isRequired } 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() diff --git a/src/core/components/operation.jsx b/src/core/components/operation.jsx index d2386521..680d2701 100644 --- a/src/core/components/operation.jsx +++ b/src/core/components/operation.jsx @@ -213,7 +213,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} /> } diff --git a/src/core/components/responses.jsx b/src/core/components/responses.jsx index f37fa8b8..46c82a69 100644 --- a/src/core/components/responses.jsx +++ b/src/core/components/responses.jsx @@ -4,18 +4,18 @@ import { fromJS, Iterable } from "immutable" import { defaultStatusCode } 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, - pathMethod: PropTypes.array.isRequired, - displayRequestDuration: PropTypes.bool.isRequired, fn: PropTypes.object.isRequired, getConfigs: PropTypes.func.isRequired } @@ -27,9 +27,26 @@ export default class Responses extends React.Component { displayRequestDuration: false } - onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue(this.props.pathMethod, val) + shouldComponentUpdate(nextProps) { + console.log("Responses SCU", this.props.tryItOutResponse.toJS(), nextProps.tryItOutResponse.toJS()) + let render = this.props.request !== nextProps.request + || 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 + + console.log("render", render) + + return render + } + + onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue([this.props.path, this.props.method], val) render() { + console.log("Responses render") let { responses, request, tryItOutResponse, getComponent, getConfigs, specSelectors, fn, producesValue, displayRequestDuration } = this.props let defaultCode = defaultStatusCode( responses ) @@ -60,7 +77,8 @@ export default class Responses extends React.Component { getComponent={ getComponent } getConfigs={ getConfigs } specSelectors={ specSelectors } - pathMethod={ this.props.pathMethod } + path={ this.props.path } + method={ this.props.method } displayRequestDuration={ displayRequestDuration } />

Responses

From cfb4625eb08059ea40e662f2ff462a4b24f14f6e Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 20 Aug 2017 18:50:49 -0600 Subject: [PATCH 2/4] Add `shouldComponentUpdate` to response.jsx and live-response.jsx. Remove `isShownKey` from `shouldComponentUpdate` of OperationContainer. Moved `response` and `request` out to separate props for operation.jsx. Removed unused `request` prop from responses.jsx. --- src/core/components/live-response.jsx | 17 +++++++++++++---- src/core/components/operation.jsx | 14 ++++++++------ src/core/components/response.jsx | 10 ++++++++-- src/core/components/responses.jsx | 16 ++++------------ src/core/containers/OperationContainer.jsx | 7 ++++--- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/core/components/live-response.jsx b/src/core/components/live-response.jsx index 457e3306..4a581d4e 100644 --- a/src/core/components/live-response.jsx +++ b/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,15 +29,24 @@ Duration.propTypes = { export default class LiveResponse extends React.Component { static propTypes = { - response: PropTypes.object.isRequired, - specSelectors: PropTypes.object.isRequired, + response: PropTypes.instanceOf(Iterable).isRequired, path: PropTypes.string.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, path, method } = this.props const { showMutatedRequest } = getConfigs() @@ -112,7 +122,6 @@ export default class LiveResponse extends React.Component { static propTypes = { getComponent: PropTypes.func.isRequired, - request: ImPropTypes.map, response: ImPropTypes.map } } diff --git a/src/core/components/operation.jsx b/src/core/components/operation.jsx index 680d2701..7e97d5d8 100644 --- a/src/core/components/operation.jsx +++ b/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, @@ -24,19 +26,21 @@ export default class Operation extends PureComponent { } static defaultProps = { - showSummary: true, + operation: null, response: null, - allowTryItOut: true, - displayOperationId: false, - displayRequestDuration: false + request: null } 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, @@ -67,8 +71,6 @@ export default class Operation extends PureComponent { tryItOutEnabled, executeInProgress } = operationProps.toJS() - let response = operationProps.get("response") - let request = operationProps.get("request") let { summary, diff --git a/src/core/components/response.jsx b/src/core/components/response.jsx index 8c0a8bf1..aca6e0d6 100644 --- a/src/core/components/response.jsx +++ b/src/core/components/response.jsx @@ -1,6 +1,6 @@ import React from "react" import PropTypes from "prop-types" -import { fromJS, Seq } from "immutable" +import { fromJS, Seq, Iterable } from "immutable" import { getSampleSchema } from "core/utils" const getExampleComponent = ( sampleResponse, examples, HighlightCode ) => { @@ -41,7 +41,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, @@ -53,6 +53,12 @@ export default class Response extends React.Component { response: fromJS({}), }; + shouldComponentUpdate(nextProps) { + return this.props.code !== nextProps.code + || this.props.response !== nextProps.response + || this.props.className !== nextProps.className + } + render() { let { code, diff --git a/src/core/components/responses.jsx b/src/core/components/responses.jsx index 46c82a69..0525fac8 100644 --- a/src/core/components/responses.jsx +++ b/src/core/components/responses.jsx @@ -5,7 +5,6 @@ import { defaultStatusCode } 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), @@ -21,33 +20,27 @@ export default class Responses extends React.Component { } static defaultProps = { - request: null, tryItOutResponse: null, produces: fromJS(["application/json"]), displayRequestDuration: false } shouldComponentUpdate(nextProps) { - console.log("Responses SCU", this.props.tryItOutResponse.toJS(), nextProps.tryItOutResponse.toJS()) - let render = this.props.request !== nextProps.request - || this.props.tryItOutResponse !== nextProps.tryItOutResponse + // 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 - - console.log("render", render) - return render } onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue([this.props.path, this.props.method], val) render() { - console.log("Responses render") - let { responses, request, tryItOutResponse, getComponent, getConfigs, specSelectors, fn, producesValue, displayRequestDuration } = this.props + let { responses, tryItOutResponse, getComponent, getConfigs, specSelectors, fn, producesValue, displayRequestDuration } = this.props let defaultCode = defaultStatusCode( responses ) const ContentType = getComponent( "contentType" ) @@ -72,8 +65,7 @@ export default class Responses extends React.Component { { !tryItOutResponse ? null :
- { @@ -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 ( Date: Sun, 20 Aug 2017 18:57:59 -0600 Subject: [PATCH 3/4] Fixed mapStateToProps test to handle non-static mapStateToProps --- test/core/system/system.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/system/system.js b/test/core/system/system.js index 67447276..2b4990df 100644 --- a/test/core/system/system.js +++ b/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" } From 6f98fa9f3b03ea05a834cf9a71d615c9adcf5d1e Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Fri, 15 Sep 2017 22:31:02 -0600 Subject: [PATCH 4/4] Fixes after merge with ft/3584 branch --- src/core/components/responses.jsx | 10 +++++----- src/core/plugins/oas3/actions.js | 4 ++-- src/core/plugins/oas3/reducers.js | 3 +-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/core/components/responses.jsx b/src/core/components/responses.jsx index 63708b2d..832201b4 100644 --- a/src/core/components/responses.jsx +++ b/src/core/components/responses.jsx @@ -15,7 +15,7 @@ export default class Responses extends React.Component { getComponent: PropTypes.func.isRequired, specSelectors: PropTypes.object.isRequired, specActions: PropTypes.object.isRequired, - oas3Actions: PropTypes.object.isRequired, + oas3Actions: PropTypes.object.isRequired, fn: PropTypes.object.isRequired, getConfigs: PropTypes.func.isRequired } @@ -38,14 +38,15 @@ export default class Responses extends React.Component { return render } - onChangeProducesWrapper = ( val ) => this.props.specActions.changeProducesValue(this.props.pathMethod, val) + 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 }) } } @@ -53,7 +54,6 @@ export default class Responses extends React.Component { render() { let { responses, - request, tryItOutResponse, getComponent, getConfigs, diff --git a/src/core/plugins/oas3/actions.js b/src/core/plugins/oas3/actions.js index ad81f3e7..c9abc855 100644 --- a/src/core/plugins/oas3/actions.js +++ b/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 } } } diff --git a/src/core/plugins/oas3/reducers.js b/src/core/plugins/oas3/reducers.js index 149f55e3..8590284d 100644 --- a/src/core/plugins/oas3/reducers.js +++ b/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 } } ) =>{ - let [path, method] = pathMethod + [UPDATE_RESPONSE_CONTENT_TYPE]: (state, { payload: { value, path, method } } ) =>{ return state.setIn( [ "requestData", path, method, "responseContentType" ], value) }, [UPDATE_SERVER_VARIABLE_VALUE]: (state, { payload: { server, key, val } } ) =>{