fix(Sticky|Visibility): add null check on window

[lottamus]

fix(Sticky|Visibility): add null check on window for server side rendering

fixes #1987

[codecov-io]

Codecov Report

Merging #1990 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1990      +/-   ##
=========================================
+ Coverage    99.8%   99.8%   +<.01%     
=========================================
  Files         148     148              
  Lines        2589    2593       +4     
=========================================
+ Hits         2584    2588       +4     
  Misses          5       5

Impacted Files	Coverage Δ
src/modules/Sticky/Sticky.js	100% <100%> (ø)	⬆️
src/behaviors/Visibility/Visibility.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23ec465...0385067. Read the comment docs.

[layershifter]

Let's use null there:

isBrowser ? window : null,

[lottamus]

@layershifter if we use null, should we also add null checks around https://github.com/ChrisNLott/Semantic-UI-React/blob/d3bd18cd7fca2be2ea5a32381b70798aa5de2c57/src/behaviors/Visibility/Visibility.js#L173? basically anywhere context is used? But I suppose the empty object would error in that case too

[lulalachen]

Looking forward to this PR! Thanks

[tim-soft]

I can't wait for this to merge, thank you!

[levithomason]

Has this been tested in SSR? I don't see how the componentDidMount would work when context is null:

  componentDidMount() {
    const { context } = this.props
    context.addEventListener('scroll', this.handleScroll)
  }

In other components, we also exit methods early if they are accessing the document or window:

  componentDidMount() {
    if (!isBrowser) return

    const { context } = this.props
    context.addEventListener('scroll', this.handleScroll)
  }

Would love to get some validation from SSR users before shipping.

[tim-soft]

I can confirm that these changes fix my SSR apps, although I also need to make the same changes in dist/commonjs/behaviors/Visibility/Visibility.js and dist/commonjs/modules/Sticky/Stick.js

[lottamus]

@levithomason I tested it against my SSR app, but I will add null checks around the lifecycle methods too

[levithomason]

@tim-soft and @ChrisNLott, thanks for investigating and reporting back. Also, to @ChrisNLott for the updates.

Merging! Will be released today :)

[levithomason]

Released in [email protected]