-
Notifications
You must be signed in to change notification settings - Fork 25
Proxy requests to webpack in development mode #145
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
Conversation
3707b57 to
34d27ce
Compare
34d27ce to
e74f12f
Compare
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.
LGTM
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.
Docs need to be updated, right?
|
@carlosms yes. I just think we need to review/test/agree on it first. And if everything is ok, I'll update docs. |
|
Trying it out and reviewing this takes some time. I'm sorry for delay @smacker, but most probably will be able to get back to this by Monday (latest). |
e74f12f to
1b3698a
Compare
|
Thinking more about this, I find it a bit strange to "pollute" the backend server with code to do proxy calls to the frontend development server. Since this is only useful for frontend development, maybe we could rethink the solution to move the changes to the webpack server. package.json accepts proxy configuration even when create-react-app is used, maybe we could proxy all This is more a team conversation starter than a change request, I though it made sense to post it here for context even if we discuss this face to face. |
|
Seems like the feedback was provided 2 weeks ago. |
|
@bzz I would like to see @dpordomingo feedback too. |
Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
1b3698a to
2036bf3
Compare
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.
Many thanks, @smacker for explaining clearly the motivations of this PR, and for taking your time to purpose this solution!!
I totally share your concerns about running the app for dev purposes (as easily as possible: with only one command, and using as less configuration as possible)
I rebased your branch against current master, tested it and it works.
After testing the tip of this branch and analyzing the proposed changes, I share the @carlosms concerns #145 (comment), plus these others:
- running go server as a background job caused problems sometimes (server didn't stop when
make devends, so next time you run it the8080port is already busy), - adding
BROWSER="none"to theyarn startcommand, and using8080port, disables the hot reloading in development: one of the most useful features of the webpack server. - some of the motivations of this PR could not be fully satisfied:
- using
make devdoes not avoid having problems with (a) "different versions of backend&frontend"; if it is changed the code of the repo whilemake devis running, the frontend is automatically updated, but the backend is not, - I do not see how the purposed changes enforce (c) "the application point of view",
- it's still needed to define the
CAT_SERVER_URLso (e) "env variables should come from BE" is not enforced
- using
Anyhow, I think this was a good starting point to solve the problems described by this PR, so I'll try to purpose an alternative based on the reasoning of this PR.
|
The project has been kind of freeze. So imo it doesn't worth to work on it anymore. |
Reasoning:
make devBased on #143