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

hot reload + api / production on 3000 to match CRA #1

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kdmcclel
Copy link

@kdmcclel kdmcclel commented May 1, 2017

I liked your approach to avoiding an eject while adding SSR / API server. I wanted to enhance development by getting the API running beside the CRA dev server. Also added in hot reloading for containers / reducers.

After I got that working for my project I thought I'd make the changes to yours and submit a pull request for anyone who finds this moving forward.

I ended up fleshing out your api enough to confirm the api server responds during development. When you go to the Second Page it fetches /api/users/1 and updates the store. Returning to the first page will show the new email address. It's pretty bare bones.

@ayroblu
Copy link
Owner

ayroblu commented May 6, 2017

Okay, so made some changes, here's my thoughts (didn't test my changes)
Proxy - cool didn't know / forgot about that one
concurrently - I actually like keeping them seperate, but that's also cool
Added api endpoint - I guess sure
Using the api - I do it a different way, so I've done it my preferred way
ApiUrl - don't need this with the proxy
Sagas - ewww so much code, but sure, I'm not sure about the syntax so I'm gonna assume that's fine
Hot reload - didn't know about this, that's cool

Once changes are tested I'll merge in

@kdmcclel
Copy link
Author

kdmcclel commented May 6, 2017

Cool looks good. Yeah I started with just concurrently / hot reloading / proxy then thought I ought to make sure the API proxy was working before submitting. Tried to follow what you had started with the commented out saga stuff.

I wasn't sure res.json() returned status at all or just the data so the saga code may have to change now.

@ayroblu
Copy link
Owner

ayroblu commented May 8, 2017

Yeah, I haven't gotten to the point where I actually need sagas. I leave the code there in case its needed cause it's common practice, but it's such a pain and 99% unnecessary

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.

2 participants