Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[287] fix undefined window #292

Closed
wants to merge 4 commits into from
Closed

Conversation

piotr-s-brainhub
Copy link
Contributor

@piotr-s-brainhub piotr-s-brainhub commented Feb 26, 2020

Deployed to https://beghp.github.io/gh-pages-rc-6

- [x] package.json:version has been updated with npm version patch (or major/minor as appropriate)
- [ ] @brainhubeu/react-carousel version has been updated in docs-www/package.json to the same which is in package.json:version

@ebello
Copy link

ebello commented Mar 4, 2020

I'm in a Gatsby environment as well and introducing window to the global state in this way will actually conflict with other plugins that check to see if window is defined.

I believe a better way would be to truly check to see if window is defined in the component itself. For example, in Carousel.js componentDidMount:

if (typeof window !== "undefined") {
    // setting size of a carousel in state
    window.addEventListener('resize', this.onResize);
    this.onResize();

    // setting size of a carousel in state based on styling
    window.addEventListener('load', this.onResize);

    // setting autoplay interval
    this.resetInterval();
}

@piotr-s-brainhub
Copy link
Contributor Author

@ebello

Thanks for your comment.
It looks that the best way for now is using Next.js and rendering Carousel on the client side.
In future maybe we'll provide server-side working - I guess the best way would be to configure webpack to generate two bundles: one for client side and another for server side.

@piotr-s-brainhub
Copy link
Contributor Author

@ebello

please see #301

@PiotrSzlagura
Copy link

For anyone looking for quick workaround, that's what I have done to use react-carousel with SSR, without Next.js:

  1. Remove all regular import Carousel, { Dots } from "@brainhubeu/react-carousel".
  2. Remove styles import from js (i had to do this as it also caused crash while compiling), import these styles inside css/scss/less instead
  3. Make sure your component contains similar checks in constructor and render:
function isBrowser() {
  return typeof window !== "undefined";
}

class MySuperSlider extends React.Component {
  constructor(props) {
    super(props);

    if (isBrowser()) {
      this.Carousel = require("@brainhubeu/react-carousel").default;
      this.Dots = require("@brainhubeu/react-carousel").Dots;
    }
  }

  render() {
    if (!isBrowser()) {
      return null;
    }

    const { Carousel, Dots } = this.

    // Do stuff using <Carousel> and <Dots> 
    // just like you would with regular import
  }
}

I do realize that this solution is far from elegant, but it gets the job done, and is easier than migrating whole project to Next.js :)

@microbit-matt-hillsdon
Copy link
Contributor

Simple approach that's inline with how things were working prior to the webpack v4 upgrade on PR #344

@RobertHebel RobertHebel deleted the 287-fix-no-window branch July 20, 2020 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants