Skip to content

Commit

Permalink
fix(Sticky): prevent npe error after unmount of component
Browse files Browse the repository at this point in the history
  • Loading branch information
fracmak committed May 30, 2018
1 parent 7e79cd3 commit def4afe
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
15 changes: 10 additions & 5 deletions src/modules/Sticky/Sticky.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,10 @@ export default class Sticky extends Component {
if (!isBrowser()) return
const { active } = this.props

if (active) this.removeListeners()
if (active) {
this.removeListeners()
cancelAnimationFrame(this.requestId)
}
}

// ----------------------------------------
Expand Down Expand Up @@ -185,7 +188,7 @@ export default class Sticky extends Component {
handleUpdate = (e) => {
if (!this.ticking) {
this.ticking = true
requestAnimationFrame(() => this.update(e))
this.requestId = requestAnimationFrame(() => this.update(e))
}
}

Expand Down Expand Up @@ -217,7 +220,7 @@ export default class Sticky extends Component {
didReachContextBottom = () => {
const { offset } = this.props

return (this.stickyRect.height + offset) >= this.contextRect.bottom
return this.stickyRect.height + offset >= this.contextRect.bottom
}

// Return true when the component reached the starting point
Expand All @@ -230,7 +233,7 @@ export default class Sticky extends Component {
didTouchScreenBottom = () => {
const { bottomOffset } = this.props

return (this.contextRect.bottom + bottomOffset) > window.innerHeight
return this.contextRect.bottom + bottomOffset > window.innerHeight
}

// Return true if the height of the component is higher than the window
Expand Down Expand Up @@ -308,7 +311,9 @@ export default class Sticky extends Component {
return (
<ElementType {...rest} className={className}>
<div ref={this.handleTriggerRef} />
<div ref={this.handleStickyRef} style={this.computeStyle()}>{children}</div>
<div ref={this.handleStickyRef} style={this.computeStyle()}>
{children}
</div>
</ElementType>
)
}
Expand Down
16 changes: 16 additions & 0 deletions test/specs/modules/Sticky/Sticky-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,5 +398,21 @@ describe('Sticky', () => {
domEvent.resize(window)
update.should.have.been.calledOnce()
})

it('is not called after unmount', (done) => {
const renderedComponent = mount(<Sticky />)
const instance = renderedComponent.instance()
const update = sandbox.spy(instance, 'update')
window.requestAnimationFrame.restore()
sandbox.stub(window, 'requestAnimationFrame').callsFake(fn => setTimeout(fn, 0))
sandbox.stub(window, 'cancelAnimationFrame').callsFake(id => clearTimeout(id))

domEvent.resize(window)
renderedComponent.unmount()
window.requestAnimationFrame(() => {
update.should.not.have.been.called()
done()
})
})
})
})

0 comments on commit def4afe

Please sign in to comment.