Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update checking for existing dotenv usage #11496

Merged
merged 3 commits into from
Mar 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions packages/next/lib/load-env-config.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,49 @@
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 })

// only do new env loading if dotenv isn't installed since we
// 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
Expand All @@ -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)
Expand All @@ -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
)
Expand Down
6 changes: 6 additions & 0 deletions test/integration/env-config-disable/app/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "env-config",
"devDependencies": {
"dotenv": "latest"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
HELLO=world
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "env-config",
"dependencies": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hi'
82 changes: 82 additions & 0 deletions test/integration/env-config-disable/test/index.test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})