Skip to content

Commit

Permalink
Simplify code, remove useless console.log, make linter pass, for redu…
Browse files Browse the repository at this point in the history 
…xjs#368
  • Loading branch information
@slorber
slorber committed May 1, 2016
1 parent 778730f commit bcffa66
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 92 deletions.
139 changes: 49 additions & 90 deletions src/components/connect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ function tryCatch(fn, ctx) {
try {
return fn.apply(ctx)
} catch (e) {
console.error(e)
errorObject.value = e
return errorObject
}
Expand All @@ -48,7 +47,6 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,

const finalMergeProps = mergeProps || defaultMergeProps
const { pure = true, withRef = false } = options
const checkMergedEquals = pure && finalMergeProps !== defaultMergeProps

// Helps track hot reloading.
const version = nextVersion++
Expand All @@ -75,22 +73,12 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,

class Connect extends Component {

shouldComponentUpdate(nextProps,nextState) {

shouldComponentUpdate() {
if ( this.skipNextRender ) {
this.skipNextRender = false;
return false;
this.skipNextRender = false
return false
}

/*
console.log("shouldComponentUpdate",!pure || nextState.mergedPropsUpdated)
console.log("shouldComponentUpdate",!pure || nextState.mergedPropsUpdated)
console.log("shouldComponentUpdate",nextState)
console.log("shouldComponentUpdate nextState.mergedPropsUpdated",nextState.mergedPropsUpdated)
*/
console.log("################## shouldComponentUpdate",nextState.mergedPropsUpdated)
console.log("################## shouldComponentUpdate",nextState)
return !pure || nextState.mergedPropsUpdated
return true
}

constructor(props, context) {
Expand Down Expand Up @@ -171,12 +159,14 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
}

trySubscribe() {
console.log("trySubscribe")
if (shouldSubscribe && !this.unsubscribe) {
this.unsubscribe = this.store.subscribe(() => {
if (!this.unsubscribe) {
return
}
if (pure && (this.store.getState() === this.state.storeState)) {
return
}
this.handleChange(false,false,true)
})
}
Expand All @@ -197,12 +187,9 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
componentWillReceiveProps(nextProps) {
if (pure) {
const propsUpdated = !shallowEqual(nextProps, this.props)
if ( propsUpdated ) {
this.handleChange(false,true,false)
}
else {
this.skipNextRender = true;
}
propsUpdated ?
this.handleChange(false,true,false) :
this.skipNextRender = true
}
else {
this.handleChange(false,true,false)
Expand All @@ -217,26 +204,19 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
clearCache() {
this.finalMapDispatchToProps = null
this.finalMapStateToProps = null
this.skipNextRender = null
}

handleChange(isInit,isPropsChange,isStateChange) {
const result = tryCatch(() => this.doHandleChange(isInit,isPropsChange,isStateChange))
handleChange(isInit,isPropsChange,isStoreStateChange) {
const result = tryCatch(() => this.doHandleChange(isInit,isPropsChange,isStoreStateChange))
if ( result === errorObject ) {
this.setState({error: errorObject.value})
this.setState({ handleChangeError: errorObject.value })
}
}

// TODO can we have a better method signature? it seems spread does not work?
doHandleChange(isInit,isPropsChange,isStateChange) {
console.log("################## handleChange for "+(isInit ? "init" : isPropsChange ? "props" : "state"))
doHandleChange(isInit,isPropsChange,isStoreStateChange) {

const storeState = this.store.getState()
if (isStateChange && pure && (storeState === this.state.storeState)) {
console.log("handleChange same state: return")
return
}

// This is put outside because it's used in both the fast and normal paths
// Put outside because it's used in both the fast and normal paths
let stateProps
let statePropsUpdated
const setStatePropsIfNeeded = (props) => {
Expand All @@ -245,66 +225,55 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
this.state.stateProps :
this.computeStateProps(props)
statePropsUpdated = !this.state.stateProps || !shallowEqual(stateProps, this.state.stateProps)
console.log("stateProps",stateProps)
console.log("statePropsUpdated",statePropsUpdated)
}
}


// Fast track: bailout early because we don't need access to fresh props here
console.log("isStateChange",isStateChange)
console.log("pure",pure)
console.log("!this.doStatePropsDependOnOwnProps",!this.doStatePropsDependOnOwnProps)
if (isStateChange && pure && !this.doStatePropsDependOnOwnProps) {
setStatePropsIfNeeded(this.props)
// Bail out early if we can
if (isStoreStateChange && pure && !this.doStatePropsDependOnOwnProps) {
setStatePropsIfNeeded(undefined) // Props not needed here
if (!statePropsUpdated) {
console.log("-> fast track return")
return
}
}



const setStateFunction = (previousState, currentProps) => {

console.log("-> normal track with props=",currentProps)

setStatePropsIfNeeded(currentProps);
const setStateFunction = (prevState, props) => {
// State props
setStatePropsIfNeeded(props)
// Dispatch props
const dispatchProps = (!isInit && pure && isPropsChange && !this.doDispatchPropsDependOnOwnProps) ?
this.state.dispatchProps :
this.computeDispatchProps(currentProps)
this.computeDispatchProps(props)
const dispatchPropsUpdated = !this.state.dispatchProps || !shallowEqual(dispatchProps, this.state.dispatchProps)
console.log("dispatchProps",dispatchProps)
console.log("dispatchPropsUpdated",dispatchPropsUpdated)

const mergedProps = (statePropsUpdated || dispatchPropsUpdated || isPropsChange) ?
computeMergedProps(stateProps, dispatchProps, currentProps) :
this.state.mergedProps
// Merged props
const mergedProps = (!statePropsUpdated && !dispatchPropsUpdated && !isPropsChange) ?
this.state.mergedProps :
computeMergedProps(stateProps, dispatchProps, props)
const mergedPropsUpdated = !this.state.mergedProps || !shallowEqual(mergedProps, this.state.mergedProps)
console.log("mergedProps",mergedProps)
console.log("mergedPropsUpdated",mergedPropsUpdated)

// From there we already know if we should call render or not
if ( pure && !mergedPropsUpdated ) {
this.skipNextRender = true
}

return {
error: undefined,
storeState,
handleChangeError: undefined,
storeState: this.store.getState(),
stateProps,
dispatchProps,
mergedProps,
mergedPropsUpdated
}
}



// See #86 and #368
// We need to avoid computng state multiple times per batch so we only run the last one
this.lastSetStateFunction = setStateFunction
this.setState((previousState, currentProps) => {
if ( this.lastSetStateFunction === setStateFunction ) {
return setStateFunction(previousState,currentProps)
}
else {
return previousState
}
});
this.setState((prevState, props) => {
const isLast = this.lastSetStateFunction === setStateFunction
return isLast ?
setStateFunction(prevState,props) :
prevState
})

}

Expand All @@ -316,24 +285,14 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
return this.refs.wrappedInstance
}

getRenderedElementProps() {
if ( withRef ) {
return {
...this.state.mergedProps,
ref: 'wrappedInstance'
}
}
else {
return this.state.mergedProps
}
}

render() {
console.log("################## render")
if ( this.state.error ) {
throw this.state.error;
if ( this.state.handleChangeError ) {
throw this.state.handleChangeError
}
return createElement(WrappedComponent,this.getRenderedElementProps())
const finalProps = withRef ?
({ ...this.state.mergedProps, ref: 'wrappedInstance' }) :
this.state.mergedProps
return createElement(WrappedComponent,finalProps)
}

}
Expand Down
2 changes: 0 additions & 2 deletions test/components/connect.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1666,8 +1666,6 @@ describe('React', () => {
const mapStateFactory = () => {
let lastProp, lastVal, lastResult
return (state, props) => {
console.log("state=",state)
console.log("props=",props)
if (props.name === lastProp && lastVal === state.value) {
memoizedReturnCount++
return lastResult
Expand Down

0 comments on commit bcffa66

Please sign in to comment.