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

react-scripts dev server: Support PROXY env var for overridable "proxy" #8598

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

Conversation

unclechu
Copy link

@unclechu unclechu commented Mar 3, 2020

Usage example:

PROXY=https://some-server.com npm start

Or as a script in package.json:

{
  "scripts": {
    "start": "react-scripts start",
    "start:staging": "PROXY=https://some-staging-server.com react-scripts start"
  },
  "proxy": "https://localhost:8888",
  "...": "..."
}

@facebook-github-bot
Copy link

Hi @unclechu!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ianschmitz
Copy link
Contributor

What is the use case? What does the current proxy solution not solve?

@unclechu
Copy link
Author

unclechu commented Mar 4, 2020

@ianschmitz As I showed in the example you can for instance have few commands to run a frontend using either local backend server or some testing/staging server. With this environment variable you don't have to change package.json contents which is usually under git index (e.g. you can accidentally commit a change of proxy field whilst you're wanted only new dependencies). Also it's inconvenient when you have to provide instructions in readmes like "if you want to run the frontend against another backend server go to some file and change some field to this specific value" while in case of this feature you could just suggest to run npm run start:staging or npm run start:local.

@unclechu
Copy link
Author

unclechu commented Mar 4, 2020

In short words: it adds more flexibility to the proxy feature.

@ianschmitz
Copy link
Contributor

You should be able to achieve your desired behavior today (two different scripts for diff environments) using environment variables within setupProxy.js.

@unclechu
Copy link
Author

unclechu commented Mar 4, 2020

@ianschmitz It required more effort to do it using setupProxy.js. Also it doesn't work the same way (proxy in package.json redirects all 404 requests to the backend while in setupProxy.js you have to provide specific base URL or many routes if endpoints are starting from root). Also the docs page is outdated, I had to debug stuff to realize that API of http-proxy-middleware has changed.

But yes, it's possible to do kind of-ish the same, with more effort and more boilerplate. You're free to close this one but I think this offers a lot more simpler way of doing this whilst adding one line of code and couple of symbols to another line is not a trade-off at all.

@ianschmitz
Copy link
Contributor

We've updated the docs page which will be published with the next release.

I'll see if there's appetite from the other maintainers. Historically the proxy has been a tricky subject with lots of weird edge cases which is why we're generally hesitant to make changes there. Hope you understand.

@stale
Copy link

stale bot commented Jul 6, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jul 6, 2020
@unclechu
Copy link
Author

unclechu commented Jul 6, 2020

Not stale bump

@stale stale bot removed the stale label Jul 6, 2020
@stale
Copy link

stale bot commented Aug 8, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Aug 8, 2020
@unclechu
Copy link
Author

unclechu commented Aug 8, 2020

Not stale bump

@stale stale bot removed the stale label Aug 8, 2020
@stale
Copy link

stale bot commented Sep 7, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Sep 7, 2020
@unclechu
Copy link
Author

unclechu commented Sep 7, 2020

Not stale bump

@stale stale bot removed the stale label Sep 7, 2020
@stale
Copy link

stale bot commented Oct 11, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Oct 11, 2020
@unclechu
Copy link
Author

Nope

@stale stale bot removed the stale label Oct 12, 2020
@stale
Copy link

stale bot commented Dec 19, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 19, 2020
@unclechu
Copy link
Author

Nah

@stale stale bot removed the stale label Dec 19, 2020
@jonjohansen
Copy link

+1

ciis0 pushed a commit to atosorigin/create-react-app that referenced this pull request Jun 3, 2021
@stale
Copy link

stale bot commented Jun 26, 2021

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jun 26, 2021
@unclechu
Copy link
Author

bump

@stale stale bot removed the stale label Jun 26, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 9, 2022
@unclechu
Copy link
Author

unclechu commented Jan 9, 2022

Nope

@stale stale bot removed the stale label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants