fix: load correct environment variables in Preview Server context

src/commands/dev/dev.ts

@@ -137,7 +137,7 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
 
   env[BLOBS_CONTEXT_VARIABLE] = { sources: ['internal'], value: encodeBlobsContext(blobsContext) }
 
-  if (!options.offline && !options.offlineEnv) {
+  if (!(options.offline || options.offlineEnv)) {
     env = await getEnvelopeEnv({ api, context: options.context, env, siteInfo })
     log(`${NETLIFYDEVLOG} Injecting environment variable values for ${chalk.yellow('all scopes')}`)
   }

src/utils/env/index.ts

@@ -8,7 +8,7 @@ import type { SiteInfo, EnvironmentVariableSource } from '../../utils/types.js'
  * These all match possible `context` values returned by the Envelope API.
  * Note that a user may also specify a branch name with the special `branch:my-branch-name` format.
  */
-export const SUPPORTED_CONTEXTS = ['all', 'production', 'deploy-preview', 'branch-deploy', 'dev'] as const
+export const SUPPORTED_CONTEXTS = ['all', 'production', 'deploy-preview', 'branch-deploy', 'dev', 'dev-server'] as const
 /**
  * Additional aliases for the user-provided env `context` option.
  */
@@ -30,7 +30,13 @@ type EnvelopeEnvVarScope =
 type EnvelopeEnvVar = Awaited<ReturnType<NetlifyAPI['getEnvVars']>>[number] & {
   scopes: EnvelopeEnvVarScope[]
 }
-type EnvelopeEnvVarContext = NonNullable<NonNullable<EnvelopeEnvVar['values']>[number]['context']>
+
+type EnvelopeEnvVarContext = NonNullable<
+  | NonNullable<EnvelopeEnvVar['values']>[number]['context']
+  // TODO(ndhoule): Netlify API is incorrect - Update OpenAPI types with this context type   ..
+  | 'dev-server'
+>
+
 export type EnvelopeEnvVarValue = {
   /**
    * The deploy context of the this env var value
@@ -104,6 +110,12 @@ export const getValueForContext = (
 ): EnvelopeEnvVarValue | undefined => {
   const isSupportedContext = (SUPPORTED_CONTEXTS as readonly string[]).includes(contextOrBranch)
   if (!isSupportedContext) {
+    // FIXME(ndhoule): If it's not a supported context, we just assume this is a branch deploy. This
+    // means that if you ever add a deploy context but forget to add it to SUPPORTED_CONTEXTS, we'll
+    // just load the wrong environment variables. (This bug is not theoretical: it's why I'm writing
+    // this comment.) We should instead pass a `context` and optional `branch` parameter to this
+    // function rather than mix the two concepts, and we should fail when an unsupported context is
+    // provided.
     const valueMatchingAsBranch = values.find((val) => val.context_parameter === contextOrBranch)
     // This is a `branch` context, which is an override, so it takes precedence
     if (valueMatchingAsBranch != null) {

tests/integration/commands/env/env-set.test.ts

@@ -196,7 +196,7 @@ describe('env:set command', async () => {
         (request) => request.method === 'PUT' && request.path === '/api/v1/accounts/test-account/env/OTHER_VAR',
       )
       expect(putRequest).toHaveProperty('body.is_secret', true)
-      expect(putRequest).toHaveProperty('body.values.length', 4)
+      expect(putRequest).toHaveProperty('body.values.length', 5)
       expect(putRequest).toHaveProperty('body.values[0].context', 'production')
       expect(putRequest).toHaveProperty('body.values[0].value', 'envelope-all-value')
       expect(putRequest).toHaveProperty('body.values[1].context', 'deploy-preview')

tests/integration/commands/env/env-unset.test.ts

@@ -72,7 +72,7 @@ describe('env:unset command', async () => {
         (request) => request.method === 'PATCH' && request.path === '/api/v1/accounts/test-account/env/OTHER_VAR',
       )
 
-      expect(patchRequests).toHaveLength(3)
+      expect(patchRequests).toHaveLength(4)
     })
   })
 

tests/unit/utils/env/index.test.ts

@@ -23,9 +23,14 @@ test('should return a matching value from a given context', () => {
       context: 'dev',
       value: 'bar',
     },
+    {
+      context: 'dev-server',
+      value: 'bar',
+    },
   ])
-  const result = getValueForContext(values, 'dev')
-  expect(result).toHaveProperty('value', 'bar')
+  expect(getValueForContext(values, 'production')).toHaveProperty('value', 'foo')
+  expect(getValueForContext(values, 'dev')).toHaveProperty('value', 'bar')
+  expect(getValueForContext(values, 'dev-server')).toHaveProperty('value', 'bar')
 })
 
 test('should return a value from the `branch` context with a matching `context_parameter` given a branch', () => {