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

Checking that the window is defined before accessing it. #66

Merged
merged 3 commits into from Aug 31, 2017
Merged

Checking that the window is defined before accessing it. #66

merged 3 commits into from Aug 31, 2017

Conversation

romellogoodman
Copy link
Contributor

Should take care of issue #65

Checking that the window is defined before accessing it so that the module doesn't break on the server.

@romellogoodman
Copy link
Contributor Author

Following the fix laid out here for universal apps.

For universal ("isomorphic") apps, prefix it with typeof window !== 'undefined' &&.

@alexreardon
Copy link
Collaborator

Looking good. I'll have a quick scan through, but i do not think we are using any other browser globals on launch.

import { applyMiddleware, createStore, compose } from 'redux';
import thunk from 'redux-thunk';
import reducer from './reducer';
import hookMiddleware from './hook-middleware';
import type { Store, Hooks } from '../types';

// eslint-disable-next-line no-underscore-dangle
const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
const composeEnhancers = typeof window === 'object'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment above stating

  1. We are checking if window is available because it might not be when server side rendering
  2. That this came from https://github.com/zalmoxisus/redux-devtools-extension#12-advanced-store-setup

@alexreardon
Copy link
Collaborator

Thanks for looking into this @romellogood !!!!

@alexreardon
Copy link
Collaborator

I have forked your example. I found it was not doing the correct SSR (it was returning undefined instead of the statically rendered application). I have checked my fork and everything is looking good.

@alexreardon
Copy link
Collaborator

Once you add the comments listed above we are good to go

@romellogoodman
Copy link
Contributor Author

Made the changes you requested!

@alexreardon alexreardon merged commit 5b9bb26 into atlassian:master Aug 31, 2017
@alexreardon
Copy link
Collaborator

🎉

I will try to do a release tomorrow afternoon which will contain this fix. Well done @romellogood !

@romellogoodman
Copy link
Contributor Author

Thanks :)

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.

None yet

2 participants