Skip to content

Refactor to allow gracefully failing if no env file provided#205

Merged
Gurpranked merged 2 commits intomainfrom
feat-graceful-fail-noenv
May 17, 2025
Merged

Refactor to allow gracefully failing if no env file provided#205
Gurpranked merged 2 commits intomainfrom
feat-graceful-fail-noenv

Conversation

@MartinMarwad
Copy link
Member

📋 PR Info

Description:

Refactored the code so that if no env file is provided, it assumes a local development environment and gracefully fails to render env value functionality. This is so quick edits can be made to the documentation without needing to touch the authentication related functionality.

Tested

Tested locally, needs to be tested with production env vars to ensure everything still works as before.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors environment variable handling to allow the app to gracefully fall back to a local development configuration when a .env file isn’t provided. Key changes include adding an env utility module to check for required env vars, updating navbar components to conditionally render based on env availability, and modifying Cognito configuration and Auth components to support local dev mode.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/utils/utils.js Added guard clauses in mobile & desktop navbar helpers
src/utils/env.js Introduced utility functions for checking and retrieving env vars
src/config/cognito-configure.js Updated configuration to fall back in local dev mode
src/config/cognito-auth-config.js Replaced direct env access with getEnvVar for consistency
src/components/Auth/index.js Bypasses auth logic when .env.local is missing

Comment on lines 16 to +20
export function useNavbarItemsMobile() {
// const { route } = useAuthenticator((context) => [context.route]);
if (!isEnvLocalLoaded()) {
// If .env.local is missing, do not render auth-related navbar items
return useThemeConfig().navbar.items;
}
Copy link

Copilot AI May 17, 2025

Choose a reason for hiding this comment

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

The check for missing env variables is repeated in both mobile and desktop navbar functions. Consider abstracting this logic into a helper function to reduce duplication.

Copilot uses AI. Check for mistakes.
}

// Defensive: Only split if OAUTH_REDIRECT_SIGN_OUT is defined
const signOutUris = process.env.OAUTH_REDIRECT_SIGN_OUT ? process.env.OAUTH_REDIRECT_SIGN_OUT.split(",") : ["", ""];
Copy link

Copilot AI May 17, 2025

Choose a reason for hiding this comment

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

For consistency with the rest of the codebase, consider using getEnvVar to access 'OAUTH_REDIRECT_SIGN_OUT' instead of directly accessing process.env.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Build check completed successfully for feat-graceful-fail-noenv!

@MartinMarwad MartinMarwad requested a review from Gurpranked May 17, 2025 20:07
Copy link
Contributor

@Gurpranked Gurpranked left a comment

Choose a reason for hiding this comment

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

LGTM, great stuff!

@Gurpranked Gurpranked merged commit 5a01498 into main May 17, 2025
1 check passed
@Gurpranked Gurpranked deleted the feat-graceful-fail-noenv branch May 17, 2025 20:59
@Gurpranked Gurpranked restored the feat-graceful-fail-noenv branch May 18, 2025 14:02
Gurpranked added a commit that referenced this pull request May 18, 2025
Gurpranked added a commit that referenced this pull request May 18, 2025
@Gurpranked
Copy link
Contributor

The core issue is that the logic checks for a configured environment file which doesn't exist on both the production and dev environment.
Just need an additional check to determine if we're running on production.

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.

3 participants