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

Remove dynamic NEXT_PUBLIC_* environment variables #3719

Merged
merged 2 commits into from
Nov 13, 2023
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
3 changes: 1 addition & 2 deletions .env-dist
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# local, heroku, stage, production
APP_ENV=local
NEXT_PUBLIC_APP_ENV=local
SERVER_URL=http://localhost:6060
PORT=6060
LOGOS_ORIGIN=
Expand Down Expand Up @@ -45,7 +44,7 @@ S3_BUCKET=
# Firefox Accounts OAuth
# leave FXA_ENABLED empty to disable FXA
FXA_ENABLED=
NEXT_PUBLIC_FXA_SETTINGS_URL=https://accounts.stage.mozaws.net/settings
FXA_SETTINGS_URL=https://accounts.stage.mozaws.net/settings

OAUTH_CLIENT_ID=edd29a80019d61a1
OAUTH_CLIENT_SECRET=get-this-from-groovecoder-or-fxmonitor-engineering
Expand Down
3 changes: 2 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
coverage
dist
scripts
scripts
sentry.*.config.ts
35 changes: 33 additions & 2 deletions sentry.client.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import * as Sentry from "@sentry/nextjs";

Sentry.init({
environment: process.env.NEXT_PUBLIC_APP_ENV,
environment: getEnvironment(),
dsn: process.env.NEXT_PUBLIC_SENTRY_DSN,

// Adjust this value in production, or use tracesSampler for greater control
Expand All @@ -23,7 +23,7 @@ Sentry.init({
// This sets the sample rate to be 10%. You may want this to be 100% while
// in development and sample at a lower rate in production
replaysSessionSampleRate: ["development", "heroku"].includes(
process.env.NODE_ENV
process.env.NODE_ENV,
)
? 1.0
: 0.1,
Expand All @@ -37,3 +37,34 @@ Sentry.init({
}),
],
});

/**
* We use the same built artifacts in every environment. Since client-side
* environment variables get compiled in at build time, the only environment
* variables that are available are the ones that are available on the build
* server, which therefore do not necessarily represent the environment the code
* will eventually run in. Therefore, we have to dynamically determine the
* environment at runtime, which is what this function does.
*/
function getEnvironment() {
if (
document.location.origin === "https://monitor.firefox.com" ||
document.location.origin === "https://monitor.mozilla.com"
) {
return "production";
}
if (
document.location.origin ===
"https://stage.firefoxmonitor.nonprod.cloudops.mozgcp.net"
) {
return "stage";
}
if (document.location.origin === "https://fx-breach-alerts.herokuapp.com") {
return "heroku";
}
if (document.location.hostname === "localhost") {
return "local";
}

return "unknown";
}
2 changes: 1 addition & 1 deletion src/app/(nextjs_migration)/(authenticated)/user/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ const MainLayout = async (props: Props) => {
</button>
<UserMenu
session={session}
fxaSettingsUrl={AppConstants.NEXT_PUBLIC_FXA_SETTINGS_URL}
fxaSettingsUrl={AppConstants.FXA_SETTINGS_URL}
nonce={getNonce()}
enabledFeatureFlags={enabledFeatureFlags}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export default async function Settings() {
</p>
<a
className="settings-link-fxa"
href={AppConstants.NEXT_PUBLIC_FXA_SETTINGS_URL}
href={AppConstants.FXA_SETTINGS_URL}
target="_blank"
rel="noopener noreferrer"
>
Expand Down
4 changes: 2 additions & 2 deletions src/app/components/client/toolbar/UserMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const UserMenu = (props: UserMenuProps) => {
<b>{props.user.email}</b>
<a
className={styles.menuItemCta}
href={process.env.NEXT_PUBLIC_FXA_SETTINGS_URL}
href={process.env.FXA_SETTINGS_URL}
ref={fxaItemRef}
rel="noopener noreferrer"
target="_blank"
Expand Down Expand Up @@ -105,7 +105,7 @@ export const UserMenu = (props: UserMenuProps) => {
>
<a
className={styles.menuItemCta}
href={process.env.NEXT_PUBLIC_EXTERNAL_SUPPORT_URL}
href={process.env.NEXT_PUBLIC_MONITOR_SUPPORT_URL}
Copy link
Collaborator Author

@Vinnl Vinnl Nov 10, 2023

Choose a reason for hiding this comment

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

This ensures it uses the value from

NEXT_PUBLIC_MONITOR_SUPPORT_URL=https://support.mozilla.org/kb/firefox-monitor
.

I've submitted a PR to remove the other version from the stage and prod machines.

ref={helpItemRef}
rel="noopener noreferrer"
target="_blank"
Expand Down
2 changes: 1 addition & 1 deletion src/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const requiredEnvVars = [
'MAX_NUM_ADDRESSES',
'MOZLOG_FMT',
'MOZLOG_LEVEL',
'NEXT_PUBLIC_FXA_SETTINGS_URL',
'FXA_SETTINGS_URL',
'NODE_ENV',
'OAUTH_ACCOUNT_URI',
'OAUTH_AUTHORIZATION_URI',
Expand Down
Loading