Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Configure hot reloading for React app during development #556

Merged
merged 3 commits into from Jun 4, 2019

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Jun 3, 2019

Overview

This PR configures hot reloading for the React app during development. To accomplish this without ejecting from CRA, I brought in

Setting this up also required adjusting our development proxy settings because the previous setup would route all requests not caught by React Router to Django -- including the websocket requests used by Webpack Dev Server to push hot updates. (This was the source, btw, of the frequent error messages about the socket disconnecting during development). To fix that I followed the guide here -- https://facebook.github.io/create-react-app/docs/proxying-api-requests-in-development#configuring-the-proxy-manually -- to set up a manual proxy.

Since not all of our API routes are prefixed with /api/, I created a list of the routes from urls.py and then looped over them to set up proxies. It seemed like this change would have a smaller impact than trying to update the Django routing here. (I think we should eventually update the Django routing, but it's out of scope for this issue.)

The result is that we now have hot reloading in development.

Connects #549

Notes

Second commit here drops an unused dependency on react-mapgl that I noticed when messing with the package.json file.

Testing Instructions

  • get branch, then ./scripts/update
  • once update has finished, visit the app at http://localhost:8081 and verify that it works as it did before
  • ./scripts/server then visit the development app at http://localhost:6543
  • make some changes to the FilterSidebarSearchTab component locally, then save. Verify that after saving the changes are rendered without refreshing the page and that the previous app state is maintained

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@jwalgran
Copy link
Contributor

jwalgran commented Jun 3, 2019

How concerned should we be about the 🚨, ☠️, and ⚠️ at the top of the rewired README?

@kellyi
Copy link
Contributor Author

kellyi commented Jun 3, 2019

How concerned should we be about the 🚨, ☠️, and ⚠️ at the top of the rewired README?

Seems like the ⚠️ is the only one that's not just decorative, and that's essentially a "you break it, you bought it" caveat. An alternative possibility for this would be that we could eject from Create React App, but we can also eject later if we encounter something insurmountable with rewired.

FWIW apparently Raster Foundry made use of react-app-rewired in a production app used internally, so there's some experience with it here already.

@rajadain
Copy link
Contributor

rajadain commented Jun 4, 2019

Taking a look now.

Copy link
Contributor

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested, works really well. Would recommend making an issue for transitioning the back-end routes so we don't forget about it.

src/app/package.json Show resolved Hide resolved
src/app/package.json Show resolved Hide resolved
@kellyi
Copy link
Contributor Author

kellyi commented Jun 4, 2019

Thanks! #558 is for the URL routes.

Kelly Innes added 3 commits June 4, 2019 13:07
- add react-app-rewired to customize Create React App config without
ejecting
- add & configure hot-reloading related libraries
- adjust development proxy to API: the prior setup was intercepting all
requests which were not caught by React Router including the websocket
requests by which webpack-dev-server hot module reloading would get
propagated. The new setup proxies an enumerated set of url paths to
match the set in Django's urls.py file
Remove react-mapgl dependency, as it is no longer used.
@kellyi kellyi force-pushed the ki/configure-hot-reloading branch from 2f7dda9 to 3035e95 Compare June 4, 2019 17:07
@kellyi
Copy link
Contributor Author

kellyi commented Jun 4, 2019

Thanks for taking a look!

@kellyi kellyi merged commit 2e8e35c into develop Jun 4, 2019
@kellyi kellyi deleted the ki/configure-hot-reloading branch June 4, 2019 17:12
path => app.use(proxy(path, djangoProxyTarget)),
);

module.exports = createProxies;
Copy link
Contributor

Choose a reason for hiding this comment

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

I see statements like

[HPM] Proxy created: /api  ->  http://django:8081

being printed to my console, but I don't see in this PR where createProxies is being called. How is this wired in to the dev server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a src/setupProxies.js file exists, the default export gets called as a function by Create React App: https://facebook.github.io/create-react-app/docs/proxying-api-requests-in-development#configuring-the-proxy-manually

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Thanks for providing that pointer to the documentation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants