Skip to content
New issue

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

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

Already on GitHub? # to your account

Use location instead of the undefined query property #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kyusu
Copy link

@kyusu kyusu commented Aug 17, 2017

Hi @richardscarrott,

I've found something which I consider a minor bug in your boilerplate. In the fetchComponentDatain the server.js you are destructuring the renderProps into query, params and history. But query is undefined. If I want to access the query parameters, I first have to get the location property and then access the query.

@kyusu
Copy link
Author

kyusu commented Aug 17, 2017

Hmm, interesting the checks fail for files I haven't touched.

@richardscarrott
Copy link
Member

richardscarrott commented Aug 17, 2017

@kyusu good catch... it also looks like history is undefined.

There doesn't appear to be much in the way of documentation for renderProps in the RR repo but I'm guessing it must have been removed when I upgraded to RR 3.x -- would you mind updating this to pass in just dispatch, params and the entire location object, e.g.

const { location, params } = renderProps;
      return (
        component
          .fetchData({
            dispatch: store.dispatch,
            location,
            params
          })
          // Make sure promise always successfully resolves
          .catch(() => {})
      );

re: CI, it looks like there's a mismatched version of prettier -- if you make the above change I'll fix that up and merge it in.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants