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

Enable localhost reverse proxy serving #3422

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

everyplace
Copy link
Contributor

This PR moves some changes from the stand-alone scenic-demo repo to the embedded version within this repo. These changes allow for running the local app behind a reverse proxy, which can be of benefit for some development environments. The changes migrate only some of the changes from PR #217, as that PR handled two things:

  1. Enable serving behind a reverse proxy
  2. Using env vars to update some varables

This smaller PR only does the first of those two tasks.

@everyplace everyplace force-pushed the feature/reverse-proxy branch from c88ecc2 to 0f9666f Compare February 14, 2024 04:46
@everyplace everyplace force-pushed the feature/reverse-proxy branch from 0f9666f to f2944bf Compare February 14, 2024 04:47
gulpfile.js Outdated
@@ -23,6 +23,7 @@ if (!isCiBuild()) {
}

const $$ = require('gulp-load-plugins')();
const dotenv = require('dotenv').config(); // eslint-disable-line
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because dotenv runs immediately to read .env files if available, it does not need to be used after creation. As this violates the eslint/no-unused-vars rule, I've disabled it for this line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would removing the const dotenv = part work too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChrisAntaki I tested this, and it still worked fine, so I've removed it in 78a6d31

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