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

fix(next/babel): read env from caller, not process #20679

Merged
merged 2 commits into from
Jan 2, 2021
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
19 changes: 15 additions & 4 deletions packages/next/build/babel/preset.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { PluginItem } from 'next/dist/compiled/babel/core'
import { dirname } from 'path'
const env = process.env.NODE_ENV
const isProduction = env === 'production'
const isDevelopment = env === 'development'
const isTest = env === 'test'

const isLoadIntentTest = process.env.NODE_ENV === 'test'
const isLoadIntentDevelopment = process.env.NODE_ENV === 'development'

type StyledJsxPlugin = [string, any] | string
type StyledJsxBabelOptions =
Expand Down Expand Up @@ -65,6 +64,18 @@ module.exports = (
): BabelPreset => {
const supportsESM = api.caller(supportsStaticESM)
const isServer = api.caller((caller: any) => !!caller && caller.isServer)
const isCallerDevelopment = api.caller((caller: any) => caller?.isDev)

// Look at external intent if used without a caller (e.g. via Jest):
const isTest = isCallerDevelopment == null && isLoadIntentTest

// Look at external intent if used without a caller (e.g. Storybook):
const isDevelopment =
isCallerDevelopment === true ||
(isCallerDevelopment == null && isLoadIntentDevelopment)

// Default to production mode if not `test` nor `development`:
const isProduction = !(isTest || isDevelopment)

const useJsxRuntime =
options['preset-react']?.runtime === 'automatic' ||
Expand Down
9 changes: 5 additions & 4 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,6 @@ export default async function getBaseWebpackConfig(
: [require('pnp-webpack-plugin')],
}

const webpackMode = dev ? 'development' : 'production'

const terserOptions: any = {
parse: {
ecma: 8,
Expand Down Expand Up @@ -947,7 +945,10 @@ export default async function getBaseWebpackConfig(
[`process.env.${key}`]: JSON.stringify(config.env[key]),
}
}, {}),
'process.env.NODE_ENV': JSON.stringify(webpackMode),
// TODO: enforce `NODE_ENV` on `process.env`, and add a test:
'process.env.NODE_ENV': JSON.stringify(
dev ? 'development' : 'production'
),
'process.env.__NEXT_CROSS_ORIGIN': JSON.stringify(crossOrigin),
'process.browser': JSON.stringify(!isServer),
'process.env.__NEXT_TEST_MODE': JSON.stringify(
Expand Down Expand Up @@ -1013,7 +1014,7 @@ export default async function getBaseWebpackConfig(
: undefined),
// stub process.env with proxy to warn a missing value is
// being accessed in development mode
...(config.experimental.pageEnv && process.env.NODE_ENV !== 'production'
...(config.experimental.pageEnv && dev
? {
'process.env': `
new Proxy(${isServer ? 'process.env' : '{}'}, {
Expand Down
11 changes: 2 additions & 9 deletions test/unit/next-babel-loader.unit.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
/* eslint-env jest */

// avoid generating __source annotations in JSX during testing:
const NODE_ENV = process.env.NODE_ENV
process.env.NODE_ENV = 'production'
require('next/dist/build/babel/preset')
process.env.NODE_ENV = NODE_ENV

function interopRequireDefault(mod) {
return mod.default || mod
}
Expand Down Expand Up @@ -370,8 +363,8 @@ describe('next-babel-loader', () => {
{ resourcePath: pageFile, isServer: false, development: true }
)

expect(output).toMatchInlineSnapshot(
`"var __jsx=React.createElement;import React from\\"react\\";export var __N_SSG=true;export default function Home(_ref){var greeting=_ref.greeting;return __jsx(\\"h1\\",null,greeting);}_c=Home;var _c;$RefreshReg$(_c,\\"Home\\");"`
expect(output).toMatch(
/var _jsxFileName="[^"]+";var __jsx=React\.createElement;import React from"react";export var __N_SSG=true;export default function Home\(_ref\)\{var greeting=_ref\.greeting;return __jsx\("h1",\{__self:this,__source:\{fileName:_jsxFileName,lineNumber:8,columnNumber:20\}\},greeting\);\}_c=Home;var _c;\$RefreshReg\$\(_c,"Home"\);/
)
})

Expand Down
9 changes: 2 additions & 7 deletions test/unit/next-babel.unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,18 @@ import { transform } from '@babel/core'

const trim = (s) => s.join('\n').trim().replace(/^\s+/gm, '')

// avoid generating __source annotations in JSX during testing:
const NODE_ENV = process.env.NODE_ENV
process.env.NODE_ENV = 'production'
const preset = require('next/dist/build/babel/preset')
process.env.NODE_ENV = NODE_ENV

const babel = (code, esm = false, presetOptions = {}, filename = 'noop.js') =>
transform(code, {
filename,
presets: [[preset, presetOptions]],
presets: [[require('next/dist/build/babel/preset'), presetOptions]],
babelrc: false,
configFile: false,
sourceType: 'module',
compact: true,
caller: {
name: 'tests',
supportsStaticESM: esm,
isDev: false,
},
}).code

Expand Down