Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

-use getEnv for isHosted check since it is client-side, geEnv() uses next-runtime-env to get the runtime envvars first before resorting to process.env
-not risky since it checks next-runtime-env and then falls back to the current way we grab hte value

Type of Change

  • Bug fix

Testing

Tested manully

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Sep 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
sim Error Error Sep 26, 2025 7:45pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Sep 26, 2025 7:45pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

Replaced direct env.NEXT_PUBLIC_APP_URL access with getEnv('NEXT_PUBLIC_APP_URL') in the isHosted check to ensure proper client-side environment variable handling.

  • Fixed environment variable access for client-side usage of NEXT_PUBLIC_APP_URL
  • Added getEnv import to support runtime environment variable resolution
  • Maintains backward compatibility with fallback to process.env
  • Ensures Docker runtime variable support through next-runtime-env

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Simple, well-understood change that improves environment variable handling with proper fallback mechanism. The change follows established patterns and addresses a legitimate client-side environment access issue.
  • No files require special attention

Important Files Changed

File Analysis

Filename        Score        Overview
apps/sim/lib/environment.ts 5/5 Correctly replaced direct env access with getEnv() for client-side NEXT_PUBLIC_APP_URL variable, ensuring proper runtime environment variable support

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Env as environment.ts
    participant GetEnv as getEnv()
    participant Runtime as next-runtime-env
    participant Process as process.env

    App->>Env: Check isHosted
    Env->>GetEnv: getEnv('NEXT_PUBLIC_APP_URL')
    GetEnv->>Runtime: runtimeEnv('NEXT_PUBLIC_APP_URL')
    alt Runtime returns value
        Runtime-->>GetEnv: Environment value
        GetEnv-->>Env: Value from runtime
    else Runtime returns undefined
        Runtime-->>GetEnv: undefined
        GetEnv->>Process: process.env['NEXT_PUBLIC_APP_URL']
        Process-->>GetEnv: Fallback value
        GetEnv-->>Env: Fallback value
    end
    Env->>Env: Compare URL with hosted domains
    Env-->>App: Boolean result (isHosted)
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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