-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Revert fetchDataDeferred behaviour on Widgets page #872
Conversation
`import { Navbar, Nav, NavItem } from 'react-bootstrap';` includes all react-bootstrap modules. By including the modules explicitly (i.e. `import Navbar from 'react-bootstrap/lib/Navbar';`), we can decrease the size of the production js-file and speed up hot reloading drastically.
docs(guide): Add 'Adding Text to Home Page' guide
Update react-bootstrap imports
remote duplicate conf
Fix link to redux-devtools
hoist-non-react-statics no longer needed
@@ -28,7 +28,10 @@ export default class Widgets extends Component { | |||
static reduxAsyncConnect(params, store) { | |||
const {dispatch, getState} = store; | |||
if (!isLoaded(getState())) { | |||
return dispatch(loadWidgets()); | |||
const promise = dispatch(loadWidgets()); | |||
if (__SERVER__) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this definitely solves the problem, but it's a patch. For example, we have them everywhere in our app and it'd be too routine and not pretty to have if (__SERVER__) {
everywhere. This does show how to solve it though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems for me, that logic of fetchDataDeferred was not really obvious...
Of course, I can add new static method , analogue of fetchDataDeferred, but i'd like to find more elegant solution...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm weird, i thought i replied to this. Sorry for the late reply. I do think a more elegant solution would be best, not sure how other than another static method that will be called after transition or componentDidMount. Or it could even be in a form of a helper? So a callback helper to be called, then youd save having two static methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought... may be for this purposes we can simply create new function that will wrap promise and return promise or undefined, depends where we are - on server or on client...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that could work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... depends where we are - on server or on client...
check window object may work in this case
const isServerSide = typeof window !== 'object';
reference PR at redux-async-connect #23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sars I really like redial's approach on defer:
https://github.com/markdalgleish/redial
Is this linked to an open issues? |
Duh.. should have read: #833 |
da8f2c2
to
e60b23c
Compare
redux-simple-router updated to react-router-redux 2.1.0
Better initialization of react router redux on server-side (with original url)
several modules updated
Migration to redux simple router and async-props
feat(ci): add support for circle-ci
docs(guide): Add 'Adding A Page' guide
e60b23c
to
b1f8dd1
Compare
No description provided.