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

Fixes #1576, Added FRONTEND env variable, updated routes/index.js #1606

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions env.example
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ NODE_ENV=development
# PORT is the port used by the web server
PORT=3000
Copy link
Contributor

@chrispinkney chrispinkney Feb 10, 2021

Choose a reason for hiding this comment

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

Does this change affect this 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.

It does, thanks for bring it up.
It seems like I need to make a conditional statement within the package.json
@humphd cc'ing just for clarity, Are we allowed to make conditionals in our packages for Telescope?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think they're related. PORT=3000 is for telescope (telescope serving the back end), The change @chrispinkney is talking about (this) is for next's production server (a separate server to serve the production build, but telescope has nothing to do with it).


Copy link
Contributor

Choose a reason for hiding this comment

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

Haha sorry, I know it's pedantic, but can you get rid of the extra newlines on line 6 and 15?


# FRONTEND is the Front-End env variable that defines what Front-End
# gets served for Telescope.
# Currently, FRONTEND only has two supported values:
# 'gatsby' - The Gatsby Front-End
# 'next' - The NextJs Front-End
# Our default value for FRONTEND is 'gatsby'
FRONTEND=gatsby


# API_URL is the URL of the Telescope Web API server. Change this to
# pick which backend server our frontend uses. If you are developing
# locally, this will be localhost:{PORT}, probably http://localhost:3000.
Expand Down
8 changes: 8 additions & 0 deletions env.production
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ NODE_ENV=production
# PORT is the port used by the web server
PORT=3000

# FRONTEND is the Front-End env variable that defines what Front-End
# gets served for Telescope.
# Currently, FRONTEND only has two supported values:
# 'gatsby' - The Gatsby Front-End
# 'next' - The NextJs Front-End
# Our default value for FRONTEND is 'gatsby'
FRONTEND=gatsby

API_URL=https://telescope.cdot.systems

# LOG_LEVEL is used to set the level of debugging for the logs.
Expand Down
10 changes: 10 additions & 0 deletions env.staging
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ NODE_ENV=production
# PORT is the port used by the web server
PORT=3000


# FRONTEND is the Front-End env variable that defines what Front-End
# gets served for Telescope.
# Currently, FRONTEND only has two supported values:
# 'gatsby' - The Gatsby Front-End
# 'next' - The NextJs Front-End
# Our default value for FRONTEND is 'gatsby'
FRONTEND=gatsby


API_URL=https://dev.telescope.cdot.systems

# LOG_LEVEL is used to set the level of debugging for the logs.
Expand Down
1 change: 1 addition & 0 deletions netlify.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
API_URL="https://dev.telescope.cdot.systems"
# We need to expose the API_URL to the front-end too
NEXT_PUBLIC_API_URL="https://dev.telescope.cdot.systems"
FRONTEND="next"
3 changes: 2 additions & 1 deletion now.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"build": {
"env": {
"API_URL": "https://dev.telescope.cdot.systems"
"API_URL": "https://dev.telescope.cdot.systems",
"FRONTEND": "gatsby"
}
},
"builds": [
Expand Down
13 changes: 11 additions & 2 deletions src/backend/web/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const user = require('./user');
const query = require('./query');

const router = express.Router();

Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out for spurious whitespace changes like this, as it churns git history unnecessarily.

/**
* In staging and production, all routes are being cached in our reverse proxy except for admin, user, health and auth.
* Please check https://github.com/Seneca-CDOT/telescope/blob/master/nginx.conf for more details about it.
Expand Down Expand Up @@ -45,9 +44,19 @@ if (process.env.NODE_ENV === 'development') {
if (process.env.PROXY_FRONTEND) {
// Allow proxying the Gatsby dev server through our backend if PROXY_FRONTEND=1 is set in env
router.use('/', createProxyMiddleware({ target: 'http://localhost:8000', changeOrigin: true }));
} else {
// Or serve the static files in the Gatsby build directory
} else if (
!process.env.FRONTEND ||
process.env.FRONTEND.toLowerCase() === 'gatsby' ||
process.env.FRONTEND === ''
) {
router.use(express.static(path.join(__dirname, '../../../frontend/gatsby/public')));
} else if (process.env.FRONTEND.toLowerCase() === 'next') {
router.use(express.static(path.join(__dirname, '../../../frontend/next/out')));
} else {
throw new Error(
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's test that this does what we want (e.g., we should probably crash right now).

`FRONTEND Value is Incorrect or Does Not Exist. Check your .env files to see if the FRONTEND value is properly set\n FRONTENDs Current Value: ${process.env.FRONTEND}`
);
}
}

Expand Down