From b307ed91868bd1d033b5a36571fcd8aa95a32b8e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 31 Mar 2020 11:27:46 -0500 Subject: [PATCH] Update checking for existing dotenv usage (#11496) * Update checking for existing dotenv usage * Check for package-lock.json also --- packages/next/lib/load-env-config.ts | 46 +++++++++-- .../env-config-disable/app/package.json | 6 ++ .../packages/sub-app/.env/.env.development | 1 + .../app/packages/sub-app/package.json | 4 + .../app/packages/sub-app/pages/index.js | 1 + .../env-config-disable/test/index.test.js | 82 +++++++++++++++++++ 6 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 test/integration/env-config-disable/app/package.json create mode 100644 test/integration/env-config-disable/app/packages/sub-app/.env/.env.development create mode 100644 test/integration/env-config-disable/app/packages/sub-app/package.json create mode 100644 test/integration/env-config-disable/app/packages/sub-app/pages/index.js create mode 100644 test/integration/env-config-disable/test/index.test.js diff --git a/packages/next/lib/load-env-config.ts b/packages/next/lib/load-env-config.ts index 7529468ada49e..9400ef7d07e65 100644 --- a/packages/next/lib/load-env-config.ts +++ b/packages/next/lib/load-env-config.ts @@ -1,12 +1,22 @@ import fs from 'fs' import path from 'path' import chalk from 'next/dist/compiled/chalk' +import findUp from 'next/dist/compiled/find-up' import dotenvExpand from 'next/dist/compiled/dotenv-expand' import dotenv, { DotenvConfigOutput } from 'next/dist/compiled/dotenv' -import findUp from 'next/dist/compiled/find-up' export type Env = { [key: string]: string } +const packageJsonHasDep = (packageJsonPath: string, dep: string): boolean => { + const { dependencies, devDependencies } = require(packageJsonPath) + const allPackages = Object.keys({ + ...dependencies, + ...devDependencies, + }) + + return allPackages.some(pkg => pkg === dep) +} + export function loadEnvConfig(dir: string, dev?: boolean): Env | false { const packageJson = findUp.sync('package.json', { cwd: dir }) @@ -14,15 +24,26 @@ export function loadEnvConfig(dir: string, dev?: boolean): Env | false { // can't check for an experimental flag in next.config.js // since we want to load the env before loading next.config.js if (packageJson) { - const { dependencies, devDependencies } = require(packageJson) - const allPackages = Object.keys({ - ...dependencies, - ...devDependencies, - }) - - if (allPackages.some(pkg => pkg === 'dotenv')) { + // check main `package.json` first + if (packageJsonHasDep(packageJson, 'dotenv')) { return false } + // check for a yarn.lock or lerna.json file in case it's a monorepo + const monorepoFile = findUp.sync( + ['yarn.lock', 'lerna.json', 'package-lock.json'], + { cwd: dir } + ) + + if (monorepoFile) { + const monorepoRoot = path.dirname(monorepoFile) + const monorepoPackageJson = path.join(monorepoRoot, 'package.json') + + try { + if (packageJsonHasDep(monorepoPackageJson, 'dotenv')) { + return false + } + } catch (_) {} + } } else { // we should always have a package.json but disable in case we don't return false @@ -49,6 +70,13 @@ export function loadEnvConfig(dir: string, dev?: boolean): Env | false { const dotEnvPath = path.join(dir, envFile) try { + const stats = fs.statSync(dotEnvPath) + + // make sure to only attempt to read files + if (!stats.isFile()) { + continue + } + const contents = fs.readFileSync(dotEnvPath, 'utf8') let result: DotenvConfigOutput = {} result.parsed = dotenv.parse(contents) @@ -62,7 +90,7 @@ export function loadEnvConfig(dir: string, dev?: boolean): Env | false { Object.assign(combinedEnv, result.parsed) } catch (err) { if (err.code !== 'ENOENT') { - console.log( + console.error( `> ${chalk.cyan.bold('Error: ')} Failed to load env from ${envFile}`, err ) diff --git a/test/integration/env-config-disable/app/package.json b/test/integration/env-config-disable/app/package.json new file mode 100644 index 0000000000000..46aaa57dea2a0 --- /dev/null +++ b/test/integration/env-config-disable/app/package.json @@ -0,0 +1,6 @@ +{ + "name": "env-config", + "devDependencies": { + "dotenv": "latest" + } +} diff --git a/test/integration/env-config-disable/app/packages/sub-app/.env/.env.development b/test/integration/env-config-disable/app/packages/sub-app/.env/.env.development new file mode 100644 index 0000000000000..940a5d8d9566a --- /dev/null +++ b/test/integration/env-config-disable/app/packages/sub-app/.env/.env.development @@ -0,0 +1 @@ +HELLO=world \ No newline at end of file diff --git a/test/integration/env-config-disable/app/packages/sub-app/package.json b/test/integration/env-config-disable/app/packages/sub-app/package.json new file mode 100644 index 0000000000000..ffaa95af60f33 --- /dev/null +++ b/test/integration/env-config-disable/app/packages/sub-app/package.json @@ -0,0 +1,4 @@ +{ + "name": "env-config", + "dependencies": {} +} diff --git a/test/integration/env-config-disable/app/packages/sub-app/pages/index.js b/test/integration/env-config-disable/app/packages/sub-app/pages/index.js new file mode 100644 index 0000000000000..0957a987fc2f2 --- /dev/null +++ b/test/integration/env-config-disable/app/packages/sub-app/pages/index.js @@ -0,0 +1 @@ +export default () => 'hi' diff --git a/test/integration/env-config-disable/test/index.test.js b/test/integration/env-config-disable/test/index.test.js new file mode 100644 index 0000000000000..9947a18e826cb --- /dev/null +++ b/test/integration/env-config-disable/test/index.test.js @@ -0,0 +1,82 @@ +/* eslint-env jest */ +/* global jasmine */ +import fs from 'fs-extra' +import { join } from 'path' +import { + nextBuild, + findPort, + launchApp, + killApp, + nextStart, + renderViaHTTP, +} from 'next-test-utils' + +jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2 + +const monorepoRoot = join(__dirname, '../app') +const yarnLock = join(monorepoRoot, 'yarn.lock') +const lernaConf = join(monorepoRoot, 'lerna.json') +const appDir = join(monorepoRoot, 'packages/sub-app') + +let app +let appPort + +const runTests = () => { + describe('dev mode', () => { + it('should start dev server without errors', async () => { + let stderr = '' + appPort = await findPort() + app = await launchApp(appDir, appPort, { + onStderr(msg) { + stderr += msg || '' + }, + }) + + const html = await renderViaHTTP(appPort, '/') + await killApp(app) + + expect(html).toContain('hi') + expect(stderr).not.toContain('Failed to load env') + }) + }) + + describe('production mode', () => { + it('should build app successfully', async () => { + const { stderr, code } = await nextBuild(appDir, [], { stderr: true }) + expect(code).toBe(0) + expect((stderr || '').length).toBe(0) + }) + + it('should start without error', async () => { + let stderr = '' + appPort = await findPort() + app = await nextStart(appDir, appPort, { + onStderr(msg) { + stderr += msg || '' + }, + }) + + const html = await renderViaHTTP(appPort, '/') + await killApp(app) + + expect(html).toContain('hi') + expect(stderr).not.toContain('Failed to load env') + }) + }) +} + +describe('Env support disabling', () => { + describe('with yarn based monorepo', () => { + beforeAll(() => fs.writeFile(yarnLock, 'test')) + afterAll(() => fs.remove(yarnLock)) + + runTests() + }) + + describe('with lerna based monorepo', () => { + beforeAll(() => fs.writeFile(lernaConf, 'test')) + afterAll(() => fs.remove(lernaConf)) + + runTests() + }) +})