From 92b4b84ca7349d6d0cf591a70b1286c310885baf Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Fri, 13 Dec 2024 14:19:38 +0100 Subject: [PATCH] fix(cli): load `.env` files before resolving options in `expo start` (#33629) # Why Fixes #33483 `expo start`'s options are quite complicated and may load the app manifest when the project is using dev clients. Because the app manifest is loaded when resolving these options, we need to move the `.env` initialization before resolving these options. > Note, we can also split up the `resolveOptions` from `expo start`, but that would still require moving the `.env` init before resolving the dev client scheme. # How - Moved the `.env` loading mechanism before resolving the start options, into `src/start/index.ts` # Test Plan See added test # Checklist - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md) --- packages/@expo/cli/CHANGELOG.md | 1 + .../@expo/cli/e2e/__tests__/start-test.ts | 38 ++++++++++++++++++- packages/@expo/cli/src/start/index.ts | 6 +++ packages/@expo/cli/src/start/startAsync.ts | 3 -- packages/@expo/cli/src/utils/nodeEnv.ts | 16 +++++++- 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/@expo/cli/CHANGELOG.md b/packages/@expo/cli/CHANGELOG.md index 5b6815e1061f30..a4c9de3043aa9a 100644 --- a/packages/@expo/cli/CHANGELOG.md +++ b/packages/@expo/cli/CHANGELOG.md @@ -14,6 +14,7 @@ - Add minor fixes to nested server actions. ([#32925](https://github.com/expo/expo/pull/32925) by [@EvanBacon](https://github.com/EvanBacon)) - Fix a build error when running `expo run:ios` consecutively without closing the app. ([#33236](https://github.com/expo/expo/pull/33236) by [@alanjhughes](https://github.com/alanjhughes)) +- Load `.env` files in `expo start` before resolving options making these env vars available in dynamic app manifests. ([#33629](https://github.com/expo/expo/pull/33629) by [@byCedric](https://github.com/byCedric)) ### 💡 Others diff --git a/packages/@expo/cli/e2e/__tests__/start-test.ts b/packages/@expo/cli/e2e/__tests__/start-test.ts index ddb2039be22557..db65ada318370d 100644 --- a/packages/@expo/cli/e2e/__tests__/start-test.ts +++ b/packages/@expo/cli/e2e/__tests__/start-test.ts @@ -97,7 +97,6 @@ describe('server', () => { await fs.promises.rm(path.join(projectRoot, '.expo'), { force: true, recursive: true }); await expo.startAsync(); }); - afterAll(async () => { await expo.stopAsync(); }); @@ -159,3 +158,40 @@ describe('server', () => { }); }); }); + +describe('start - dev clients', () => { + const expo = createExpoStart({ + env: { + EXPO_USE_FAST_RESOLVER: 'true', + }, + }); + + beforeAll(async () => { + const projectRoot = await setupTestProjectWithOptionsAsync('start-dev-clients', 'with-blank'); + expo.options.cwd = projectRoot; + + // Add a `.env` file with `TEST_SCHEME` + await fs.promises.writeFile(path.join(projectRoot, '.env'), `TEST_SCHEME=some-value`); + // Add a `app.config.js` that asserts an env var from .env + await fs.promises.writeFile( + path.join(projectRoot, 'app.config.js'), + `const assert = require('node:assert'); + const { env } = require('node:process'); + + module.exports = ({ config }) => { + assert(env.TEST_SCHEME, 'TEST_SCHEME is not defined'); + return { ...config, scheme: env.TEST_ENV }; + };` + ); + + await expo.startAsync(['--dev-client']); + }); + afterAll(async () => { + await expo.stopAsync(); + }); + + it('runs `npx expo start` in dev client mode, using environment variable from .env', async () => { + const response = await expo.fetchBundleAsync('/'); + expect(response.ok).toBeTruthy(); + }); +}); diff --git a/packages/@expo/cli/src/start/index.ts b/packages/@expo/cli/src/start/index.ts index 170333194562ce..c55d92db8721f4 100644 --- a/packages/@expo/cli/src/start/index.ts +++ b/packages/@expo/cli/src/start/index.ts @@ -82,6 +82,12 @@ export const expoStart: Command = async (argv) => { } const projectRoot = getProjectRoot(args); + + // NOTE(cedric): `./resolveOptions` loads the expo config when using dev clients, this needs to be initialized before that + const { setNodeEnv, loadEnvFiles } = await import('../utils/nodeEnv.js'); + setNodeEnv(!args['--no-dev'] ? 'development' : 'production'); + loadEnvFiles(projectRoot); + const { resolveOptionsAsync } = await import('./resolveOptions.js'); const options = await resolveOptionsAsync(projectRoot, args).catch(logCmdError); diff --git a/packages/@expo/cli/src/start/startAsync.ts b/packages/@expo/cli/src/start/startAsync.ts index 65a5ece9ad5c6e..5c787e41c21a4e 100644 --- a/packages/@expo/cli/src/start/startAsync.ts +++ b/packages/@expo/cli/src/start/startAsync.ts @@ -14,7 +14,6 @@ import { openPlatformsAsync } from './server/openPlatforms'; import { getPlatformBundlers, PlatformBundlers } from './server/platformBundlers'; import { env } from '../utils/env'; import { isInteractive } from '../utils/interactive'; -import { setNodeEnv } from '../utils/nodeEnv'; import { profile } from '../utils/profile'; async function getMultiBundlerStartOptions( @@ -68,8 +67,6 @@ export async function startAsync( ) { Log.log(chalk.gray(`Starting project at ${projectRoot}`)); - setNodeEnv(options.dev ? 'development' : 'production'); - require('@expo/env').load(projectRoot); const { exp, pkg } = profile(getConfig)(projectRoot); if (exp.platforms?.includes('ios') && process.platform !== 'win32') { diff --git a/packages/@expo/cli/src/utils/nodeEnv.ts b/packages/@expo/cli/src/utils/nodeEnv.ts index c4232d9b6f0bd5..fc4731768dd811 100644 --- a/packages/@expo/cli/src/utils/nodeEnv.ts +++ b/packages/@expo/cli/src/utils/nodeEnv.ts @@ -1,5 +1,9 @@ -// Set the environment to production or development -// lots of tools use this to determine if they should run in a dev mode. +import * as env from '@expo/env'; + +/** + * Set the environment to production or development + * lots of tools use this to determine if they should run in a dev mode. + */ export function setNodeEnv(mode: 'development' | 'production') { process.env.NODE_ENV = process.env.NODE_ENV || mode; process.env.BABEL_ENV = process.env.BABEL_ENV || process.env.NODE_ENV; @@ -7,3 +11,11 @@ export function setNodeEnv(mode: 'development' | 'production') { // @ts-expect-error: Add support for external React libraries being loaded in the same process. globalThis.__DEV__ = process.env.NODE_ENV !== 'production'; } + +/** + * Load the dotenv files into the current `process.env` scope. + * Note, this requires `NODE_ENV` being set through `setNodeEnv`. + */ +export function loadEnvFiles(projectRoot: string, options?: Parameters[1]) { + return env.load(projectRoot, options); +}