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

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Nov 10, 2023

References:

Jira: MNTOR-2434 and MNTOR-2145
Figma: N/A

Description

It's fine to use NEXT_PUBLIC_* environment variables that are defined in .env-dist and are the same in every environment. However, since they are inserted at build time, they cannot be dynamically inserted at runtime, and trying to do so will instead inject the value they have at build time.

I'll also submit a PR to remove those values from the prod env.

@Vinnl Vinnl requested review from rhelmer and flozia November 10, 2023 13:11
@Vinnl Vinnl added the Review: µ Code review time: 5 minutes or less label Nov 10, 2023
@@ -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.

It's fine to use NEXT_PUBLIC_* environment variables that are
defined in `.env-dist` and are the same in every environment.
However, since they are inserted at build time, they cannot be
dynamically inserted at runtime, and trying to do so will instead
inject the value they have at build time.
@Vinnl Vinnl force-pushed the MNTOR-2434-no_NEXT_PUBLIC branch from d1a7ce6 to 20ff0b1 Compare November 10, 2023 13:17
@Vinnl Vinnl self-assigned this Nov 10, 2023
@Vinnl Vinnl merged commit efec9ae into main Nov 13, 2023
10 checks passed
@Vinnl Vinnl deleted the MNTOR-2434-no_NEXT_PUBLIC branch November 13, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: µ Code review time: 5 minutes or less
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants