Improve environment determination and site url fetching to be well tested and consistent

Author: webstackdev

@types/window.d.ts

@@ -22,11 +22,9 @@ declare global {
     metaColors?: MetaColors
 
     /**
-     * Test flag to force error throwing in development
-     * Used by error handler to re-throw errors for debugging
-     * Set by E2E tests via page.evaluate()
+     * Test flag to indicate Playwright control
      */
-    _throw?: boolean
+    isPlaywrightControlled?: boolean
   }
 }
 

astro.config.ts

@@ -8,9 +8,19 @@ import AstroPWA from '@vite-pwa/astro'
 import vtbot from 'astro-vtbot'
 import icon from 'astro-icon'
 import { defineConfig } from 'astro/config'
+/**
+ * You cannot use path aliases (@lib, @components, etc.) in files that are
+ * imported by astro.config.ts, because the path alias resolution happens
+ * AFTER the config is loaded, not before. This means that adding resolve.alias
+ * paths to the vite section in astro.config.ts would not allow using path
+ * aliases because it creates a circular dependency problem since the config
+ * file itself is importing from the paths it needs to configure.
+ */
 import {
   environmentalVariablesConfig,
+  getSentryAuthToken,
   getSiteUrl,
+  isVercel,
   markdownConfig,
   serviceWorkerConfig,
   vercelConfig,
@@ -19,13 +29,6 @@ import { callToActionValidator } from './src/integrations/CtaValidator'
 import { privacyPolicyVersion } from './src/integrations/PrivacyPolicyVersion'
 import { createSerializeFunction, pagesJsonWriter } from './src/integrations/sitemapSerialize'
 
-// Type guard for required environment variables (only in Vercel)
-const IS_VERCEL = process.env['VERCEL']
-const SENTRY_AUTH_TOKEN = process.env['SENTRY_AUTH_TOKEN']
-if (IS_VERCEL && !SENTRY_AUTH_TOKEN) {
-  throw new Error('SENTRY_AUTH_TOKEN environment variable is required in Vercel but not set')
-}
-
 export default defineConfig({
   adapter: vercelStatic(vercelConfig),
   devToolbar: {
@@ -45,10 +48,10 @@ export default defineConfig({
     /** Inject privacy policy version from git commit date for GDPR record keeping */
     privacyPolicyVersion(),
     /** Only include Sentry integration in Vercel environments */
-    ...(IS_VERCEL ? [sentry({
+    ...(isVercel() ? [sentry({
       project: "webstack-builders-corporate-website",
       org: "webstack-builders",
-      authToken: SENTRY_AUTH_TOKEN!, // Non-null assertion safe due to check above
+      authToken: getSentryAuthToken(),
     })] : []),
     sitemap({
       serialize: createSerializeFunction({

docs/ENVIRONMENT_VARIABLES

@@ -0,0 +1,72 @@
+# Environment variables
+
+## CI / CD
+
+Install Vercel CLI if not already done. Linking it will create a .vercel directory in the project with local configuration details. This directory is gitignored.
+
+```bash
+npm install -g vercel
+vercel link
+```
+
+Pull env vars from Vercel:
+
+```bash
+vercel env pull .env
+```
+
+See [Deployment](./DEPLOYMENT_SETUP.md) for information on adding environment variables to the GitHub repository for use in workflow Actions.
+
+## Loading
+
+Astro uses Vite to load environment variables, which in turn uses `dotenv` to load them from several `.env` files located in the project's root directory. The specific files loaded, and their priority, depend on the current mode (development or production). All `.env*` files are gitignored.
+
+- `.env`: Loaded in all cases, providing general environment variables.
+- `.env.local`: Loaded in all cases, intended for local development settings.
+- `.env.[mode]`: Loaded only in the specified mode (`.env.development` when running the Astro dev server, and `.env.production` when running `astro build`).
+- `.env.[mode].local`: Loaded only in the specified mode, providing local overrides for a specific mode.
+
+## Availability
+
+1. Auto-loaded environment variables are not available in **config files**. The files are excluded from the ESLint linting rule forbidding `process.env`. They can use Vite's `loadEnv()` method to load `.env*` files.
+2. All environment variables are available in **server-side code**.
+3. Only environment variables prefixed with `PUBLIC_` are available in **client-side code**. Vite extracts these variables from `.env*` files during loading and bundles them into the client script.
+
+## Typing
+
+There are two ways in an Astro project to type environmental variables in Astro:
+
+1. Add them to `src/lib/config/environmentalVariableValidation.ts`. This provides validation for the variables. Astro will generate a `.astro/env.d.ts` file with the types exported as `const` from `astro sync`, ran automatically on `astro dev` and `astro build`. They will be available as tokens from `astro:env/server` and `astro:env/client` respectively.
+2. Add them to `src/env.d.ts`. This will provide TypeScript typing for intellisense but no validation. Environmental variables added this way will not be made available on the `astro:env` imports.
+
+## Use in Code
+
+Import environment variables defined in `environmentalVariableValidation` directly.
+
+All variables:
+
+```typescript
+import { ENV_VAR } from 'astro:env/server'
+```
+
+Client side variables:
+
+```typescript
+import { PUBLIC_ENV_VAR } from 'astro:env/client'
+```
+
+The `process.env` and `import.meta.env` methods of accessing environment variables will generate linting errors if used outside of the root-level configuration files where they are necessary.
+
+## Determining Site URL
+
+The site URL can vary based on whether code is being ran from the Astro dev server, being ran in production (including being ran with `astro serve`), being ran from Vitest in unit files,
+
+- Use the `getSiteUrl()` method from `src/components/scripts/utils/siteUrlClient.ts` to determine the site URL in **client code**.
+- Use the `getSiteUrl()` method from `src/lib/config/siteUrlServer.ts` to determine the site URL in **server code**.
+
+## Determining Mode
+
+Use well tested methods to determine what mode (dev, prod, ci, vitest, playwright) code is operating in:
+
+- Use methods in `src/components/scripts/utils/environmentClient.ts` to determine mode in **client code**.
+- Use  methods in `src/lib/config/environmentServer.ts` to determine mode in **server code**.

eslint.config.ts

@@ -159,7 +159,7 @@ export default [
       'no-new': level,
       'no-process-env': [level, , {
         "message": "Do not use  process.env directly. See docs/ENVIRONMENT_VARIABLES."
-      }]
+      }],
       'no-restricted-globals': ['error'].concat(restrictedGlobals),
       'no-restricted-imports': [
         'error', {
@@ -199,14 +199,14 @@ export default [
   {
     files: ['src/components/scripts/assertions/elements.ts'],
     rules: {
-      // This file implements type guards and legitimately needs type assertions
+      /** This file implements type guards and legitimately needs type assertions */
       'custom-rules/no-html-element-assertions': 'off',
     },
   },
   {
     files: ['**/*error.spec.ts'],
     rules: {
-      // Error test files use mock objects which legitimately need type assertions
+      /** Error test files use mock objects which legitimately need type assertions */
       'custom-rules/no-html-element-assertions': 'off',
     },
   },
@@ -221,7 +221,7 @@ export default [
   },
   {
     files: [
-      // Database files that use snake_case for database field names
+      /** Database files that use snake_case for database field names */
       'src/components/scripts/consent/db/__tests__/rls.spec.ts',
       'src/pages/api/gdpr/consent.ts',
       'src/pages/api/gdpr/request-data.ts',
@@ -233,14 +233,14 @@ export default [
     },
   },
   {
-    // Test files can import from server code in src/lib
+    /** Test files can import from server code in src/lib */
     files: ['src/**/__tests__/**/*'],
     rules: {
       'import/no-restricted-paths': 'off',
     },
   },
   {
-    // Test files can import from server code in src/lib
+    /** Test files can import from server code in src/lib */
     'import/no-restricted-paths': [
       level,
       {
@@ -260,7 +260,7 @@ export default [
     ],
   },
   {
-    // Files that need access to process.env()
+    /** These directories should can use process.env, which is forbidden in other files */
     files: [
       '.eslintrc.js',
       'astro.config.ts',
@@ -273,6 +273,7 @@ export default [
     },
   },
   {
+    /** No import.meta.env use in general. */
     files: [
       'src/components/**/*',
       'src/layouts/**/*',
@@ -289,6 +290,7 @@ export default [
     },
   },
   {
+    /** These files can use import.meta.env */
     files: [
       'src/lib/config/environmentServer.ts',
       'src/lib/config/siteUrlServer.ts',
@@ -304,4 +306,25 @@ export default [
       ],
     },
   },
+  {
+    /**
+     * Path aliases cannot be used in files that are imported by astro.config.ts.
+     */
+    files: [
+      'src/integrations/**/*',
+      'src/lib/config/**/*',
+    ],
+    rules: {
+      'no-restricted-imports': [
+        'error', {
+            patterns: [
+              {
+                group: ['@/*'],
+                message: 'Path aliases cannot be used in files that are imported by astro.config.ts. See notes in that config file for the reasons why.',
+              },
+            ],
+          },
+      ],
+    },
+  },
 ]

src/components/scripts/utils/__tests__/environmentClient.spec.ts

@@ -0,0 +1,29 @@
+// @vitest-environment happy-dom
+/**
+ * Unit tests for environment detection utilities
+ * Tests the environment detection functions in a Vitest/happy-dom context
+ */
+import { describe, it, expect } from 'vitest'
+import { isUnitTest, isE2eTest, isTest, isDev, isProd } from '@components/scripts/utils'
+
+describe('Environment detection utilities', () => {
+  it('isUnitTest should return true when running in Vitest', () => {
+    expect(isUnitTest()).toBe(true)
+  })
+
+  it('isE2eTest should return false in unit test context (no Playwright control)', () => {
+    expect(isE2eTest()).toBe(false)
+  })
+
+  it('isTest should return true when ran in unit test', () => {
+    expect(isTest()).toBe(true)
+  })
+
+  it('isDev should return true when ran in unit test', () => {
+    expect(isDev()).toBe(true)
+  })
+
+  it('isProd should return false when ran in unit test', () => {
+    expect(isProd()).toBe(false)
+  })
+})

src/components/scripts/utils/environmentClient.ts

@@ -1,26 +1,27 @@
+
 /**
- * Client-side checks for environment
+ * We are setting the VITEST env var in vitest.config.ts for unit tests.
  */
-
-// @TODO: Implement these methods
-
-export const getSiteUrl = () => {
-  return "localhost:4321"
+export const isUnitTest = () => {
+  return typeof process !== 'undefined' && !!process.env['VITEST']
 }
 
-export const isCI = () => {
-  return true
+export const isE2eTest = () => {
+  // In browser context (E2E tests with Playwright)
+  if (typeof window !== 'undefined') {
+    return window.isPlaywrightControlled === true
+  }
+  return false
 }
 
-// Vitest sets the VITEST environment variable and exposes them from your .env files as import.meta.env.
-export const isUnitTest = () => {
-  return true
+export const isTest = () => {
+  return isUnitTest() || isE2eTest()
 }
 
 export const isDev = () => {
-  return true
+  return import.meta.env.DEV
 }
 
 export const isProd = () => {
-  return true
+  return import.meta.env.PROD && !isUnitTest()
 }

src/components/scripts/utils/index.ts

@@ -2,8 +2,6 @@
  * Scripts Utils - Barrel Export
  * Single source for all utility functions
  */
-
-// Re-export cookie utilities
 export {
   type CookieOptions,
   getCookie,
@@ -12,17 +10,18 @@ export {
   getAllCookies,
 } from './cookies'
 
-// Re-export data subject ID utilities
 export {
   getOrCreateDataSubjectId,
   deleteDataSubjectId,
 } from './dataSubjectId'
 
-// Re-export cookie utilities
 export {
-  getSiteUrl,
   isCI,
   isUnitTest,
+  isE2eTest,
+  isTest,
   isDev,
   isProd,
 } from './environmentClient'
+
+export { getSiteUrl} from './siteUrlClient'

src/components/scripts/utils/siteUrlClient.ts

@@ -0,0 +1,18 @@
+/**
+ * Client-side method to determine correct URL
+ */
+
+import { DEV_SERVER_PORT } from "astro:env/client"
+import { isProd, isE2eTest } from '@components/scripts/utils/environmentClient'
+
+export const getSiteUrl = () => {
+  if (isProd() && !isE2eTest()) {
+    console.log(
+      `Using production environment with domain from astro config: ${import.meta.env.SITE}`
+    )
+    return `https://${import.meta.env.SITE}`
+  } else {
+    console.log(`Using development environment on port ${DEV_SERVER_PORT}.`)
+    return `http://localhost:${DEV_SERVER_PORT}`
+  }
+}

src/lib/config/__tests__/environmentServer.spec.ts

@@ -0,0 +1,34 @@
+/**
+ * Unit tests for cookie utilities
+ * Tests the wrapper functions around js-cookie library
+ */
+import { describe, it, expect } from 'vitest'
+import {
+  isDev,
+  isE2eTest,
+  isProd,
+  isTest,
+  isUnitTest,
+} from '@lib/config'
+
+describe('Build-time environment handling utilities', () => {
+  it('should always report running in Vitest in a unit test', () => {
+    expect(isUnitTest()).toBeTruthy()
+  })
+
+  it('should report not running in Playwright in a unit test', () => {
+    expect(isE2eTest()).toBeFalsy()
+  })
+
+  it('should report isTest is true when running in Vitest', () => {
+    expect(isTest()).toBeTruthy()
+  })
+
+  it('should report running in development environment when running in Vitest', () => {
+    expect(isDev()).toBeTruthy()
+  })
+
+  it('should report not running in production environment when running in Vitest', () => {
+    expect(isProd()).toBeFalsy()
+  })
+})