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

fix: fe crash due to referencing server env var NODE_ENV #384

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

zhongliang02
Copy link
Contributor

Context

FE was crashing due to referencing server env var NODE_ENV in src/pages/_app.tsx

This issue comes from PR #383 where we replaced NODE_ENV references.

Root cause

  1. Failure to differentiate between env.NODE_ENV and process.env.NODE_ENV:
  • env.NODE_ENV is evaluated at runtime
  • process.env.NODE_ENV is baked during build time.
  1. Human factors:

Approach & Testing

  • Corrected all NODE_ENV references to the appropriate types (runtime vs build time)
  • Tested locally with npm run dev and npm run build && npm run start, works perfectly.

Copy link

vercel bot commented Jan 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
starter-kit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 3, 2025 5:05am

@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 3, 2025

Datadog Report

Branch report: 01-03-fix_fe_crash_due_to_referencing_server_env
Commit report: b4ca68a
Test service: starter-kit

✅ 0 Failed, 11 Passed, 0 Skipped, 10.23s Total Time
➡️ Test Sessions change in coverage: 1 no change

@zhongliang02 zhongliang02 requested a review from karrui January 3, 2025 05:08
@zhongliang02 zhongliang02 merged commit 17f952a into main Jan 3, 2025
14 checks passed
@zhongliang02 zhongliang02 deleted the 01-03-fix_fe_crash_due_to_referencing_server_env branch January 3, 2025 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants