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

NextJS: add /about and /projects base paths #2519

Merged
merged 3 commits into from
Nov 10, 2021
Merged
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
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ services:
command: ["start"]
environment:
- PORT=3000
- PROJECT_ASSET_PREFIX=http://localhost:3000
- PROJECT_ASSET_PREFIX=http://localhost:3000/projects
- NODE_ENV=production
- PANOPTES_ENV=production
- NEXT_TELEMETRY_DISABLED=1
Expand All @@ -39,7 +39,7 @@ services:
command: ["start"]
environment:
- PORT=3000
- CONTENT_ASSET_PREFIX=http://localhost:3001
- CONTENT_ASSET_PREFIX=http://localhost:3001/about
camallen marked this conversation as resolved.
Show resolved Hide resolved
- NODE_ENV=production
- PANOPTES_ENV=production
- NEXT_TELEMETRY_DISABLED=1
Expand Down
4 changes: 2 additions & 2 deletions kubernetes/deployment-production.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ spec:
- containerPort: 3000
env:
- name: PROJECT_ASSET_PREFIX
value: https://fe-project.zooniverse.org
value: https://fe-project.zooniverse.org/projects
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be changed to https://www.zooniverse.org/projects, once routing is set up.

- name: COMMIT_ID
value: __IMAGE_TAG__
- name: NEWRELIC_LICENSE_KEY
Expand Down Expand Up @@ -153,7 +153,7 @@ spec:
- containerPort: 3000
env:
- name: CONTENT_ASSET_PREFIX
value: https://fe-content-pages.zooniverse.org
value: https://fe-content-pages.zooniverse.org/about
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can change to https://www.zooniverse.org/about.

- name: COMMIT_ID
value: __IMAGE_TAG__
- name: CONTENTFUL_ACCESS_TOKEN
Expand Down
4 changes: 2 additions & 2 deletions kubernetes/deployment-staging.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ spec:
- containerPort: 3000
env:
- name: PROJECT_ASSET_PREFIX
value: https://fe-project.preview.zooniverse.org
value: https://fe-project.preview.zooniverse.org/projects
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can change to https://frontend.preview.zooniverse.org/projects.

- name: COMMIT_ID
value: __IMAGE_TAG__
- name: NEWRELIC_LICENSE_KEY
Expand Down Expand Up @@ -135,7 +135,7 @@ spec:
- containerPort: 3000
env:
- name: CONTENT_ASSET_PREFIX
value: https://fe-content-pages.preview.zooniverse.org
value: https://fe-content-pages.preview.zooniverse.org/about
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can change to https://frontend.preview.zooniverse.org/about.

- name: COMMIT_ID
value: __IMAGE_TAG__
- name: CONTENTFUL_ACCESS_TOKEN
Expand Down
1 change: 1 addition & 0 deletions packages/app-content-pages/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const COMMIT_ID = process.env.COMMIT_ID || commitID()

const nextConfig = {
assetPrefix,
basePath: '/about',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we allow this to be modified by ENV var with the default /about as fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. It changes the URL of all the pages served by the app and I'm not sure I see the value of changing those from /about/team and /about/publications to a different URL.


env: {
COMMIT_ID,
Expand Down
1 change: 0 additions & 1 deletion packages/app-content-pages/pages/about/publications.js

This file was deleted.

3 changes: 0 additions & 3 deletions packages/app-content-pages/pages/about/team.js

This file was deleted.

1 change: 1 addition & 0 deletions packages/app-content-pages/pages/publications.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, getStaticProps } from '../src/screens/Publications'
3 changes: 3 additions & 0 deletions packages/app-content-pages/pages/team.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { default, getStaticProps } from '../src/screens/Team'


4 changes: 3 additions & 1 deletion packages/app-project/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ function commitID () {
}
}

const basePath = '/projects'
Copy link
Contributor

Choose a reason for hiding this comment

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

Again - any use in allowing this to be set via ENV var with the above as fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here. I'm not sure about the value of changing project URLs away from /projects/:owner:/:project:, with the associated overhead of updating our static setup.

const PANOPTES_ENV = process.env.PANOPTES_ENV || 'staging'
const webpackConfig = require('./webpack.config')
const assetPrefix = process.env.PROJECT_ASSET_PREFIX || ''
const assetPrefix = process.env.PROJECT_ASSET_PREFIX || basePath
const SENTRY_PROJECT_DSN = process.env.SENTRY_PROJECT_DSN
const APP_ENV = process.env.APP_ENV || 'production'
const COMMIT_ID = process.env.COMMIT_ID || commitID()
Expand All @@ -30,6 +31,7 @@ console.info(PANOPTES_ENV, talkHosts[PANOPTES_ENV])

const nextConfig = {
assetPrefix,
basePath,

env: {
COMMIT_ID,
Expand Down