Skip to content

Allow dotenv files from different environments for SSR and SSG in Next.js#7323

Merged
chalosalvador merged 14 commits intomasterfrom
chalosalvador/dotenv-files
Aug 14, 2024
Merged

Allow dotenv files from different environments for SSR and SSG in Next.js#7323
chalosalvador merged 14 commits intomasterfrom
chalosalvador/dotenv-files

Conversation

@chalosalvador
Copy link
Copy Markdown
Member

Description

This PR is intended to fix the issue #7207 by checking if the .env. file exists and make it available for the build process.

Scenarios Tested

  1. Initialized a hosting project with Next.js
  2. Created a .env.<PROJECT-ID> file in the root of the Next.js project
  3. Created a page that uses SSR and accesses the environment variable in .env.<PROJECT-ID>
  4. Created a page that uses SSG and accesses the environment variable in .env.<PROJECT-ID>
  5. Deployed the project to Firebase hosting.
  6. Make sure the environment variable in the .env.<PROJECT-ID> is available in a server side rendered page.
  7. Make sure the environment variable in the .env.<PROJECT-ID> is available in a static generated page.

Sample Commands

Copy link
Copy Markdown
Member

@jamesdaniels jamesdaniels left a comment

Choose a reason for hiding this comment

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

Can we add some test coverage?

const projectEnvVars = parseStrict((await readFile(projectEnvPath)).toString());

// Merge the parsed variables with the existing environment variables
Object.assign(env, projectEnvVars);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dotenv does not override environment variables by default if i recall, so this logic will likely need inverting { ...projectEnvVars, ...process.env }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jamesdaniels I just made this change, added tests for the parseStrict function and added the changelog.

Copy link
Copy Markdown
Member

@jamesdaniels jamesdaniels left a comment

Choose a reason for hiding this comment

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

See small comment on overrides, add tests, and changelog—otherwise LGTM

@chalosalvador chalosalvador enabled auto-merge (squash) August 14, 2024 08:56
@chalosalvador chalosalvador merged commit 35c0051 into master Aug 14, 2024
@chalosalvador chalosalvador deleted the chalosalvador/dotenv-files branch August 14, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants