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.

Sign up for GitHub

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

Jump to bottom

Refactor define-env-plugin to have stricter types #63128

Merged
merged 5 commits into from
Mar 11, 2024
Merged

Refactor define-env-plugin to have stricter types #63128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd5e9b2
Refactor define-env-plugin to have stricter types
timneutkens Mar 11, 2024
a4eb7d0
Fix hardcoded value
timneutkens Mar 11, 2024
bcc2b73
Ensure undefined doesn't accidentally slip through
timneutkens Mar 11, 2024
4eac0c9
Remove value check
timneutkens Mar 11, 2024
c1684af
Use NextConfigComplete type
timneutkens Mar 11, 2024
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
326 changes: 176 additions & 150 deletions packages/next/src/build/webpack/plugins/define-env-plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import type { NextConfigComplete } from '../../../server/config-shared'
import type {
I18NDomains,
NextConfigComplete,
} from '../../../server/config-shared'
import type { MiddlewareMatcher } from '../../analysis/get-page-static-info'
import { webpack } from 'next/dist/compiled/webpack/webpack'
import { needsExperimentalReact } from '../../../lib/needs-experimental-react'
Expand All @@ -14,16 +17,16 @@ function errorIfEnvConflicted(config: NextConfigComplete, key: string) {
}
}

type BloomFilter = ReturnType<
import('../../../shared/lib/bloom-filter').BloomFilter['export']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not import at the top?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export is a method of BloomFilter, theoretically we could import BloomFilter at the top but didn't want to make too many changes, kept it as-is from this line: https://github.com/vercel/next.js/pull/63128/files#diff-28c0329619b90836564a7769c43ff473dbd28e8e31b75e650624da4d06911763L22 just sharing the type more.

>

export interface DefineEnvPluginOptions {
isTurbopack: boolean
allowedRevalidateHeaderKeys: string[] | undefined
clientRouterFilters?: {
staticFilter: ReturnType<
import('../../../shared/lib/bloom-filter').BloomFilter['export']
>
dynamicFilter: ReturnType<
import('../../../shared/lib/bloom-filter').BloomFilter['export']
>
staticFilter: BloomFilter
dynamicFilter: BloomFilter
}
config: NextConfigComplete
dev: boolean
Expand All @@ -38,6 +41,92 @@ export interface DefineEnvPluginOptions {
previewModeId: string | undefined
}

interface DefineEnv {
[key: string]:
| string
| string[]
| boolean
| undefined
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
| MiddlewareMatcher[]
| BloomFilter
| Partial<NextConfigComplete['images']>
| I18NDomains
}

interface SerializedDefineEnv {
[key: string]: string
}

/**
* Collects all environment variables that are using the `NEXT_PUBLIC_` prefix.
*/
function getNextPublicEnvironmentVariables(): DefineEnv {
const defineEnv: DefineEnv = {}
for (const key in process.env) {
if (key.startsWith('NEXT_PUBLIC_')) {
const value = process.env[key]
if (value) {
defineEnv[`process.env.${key}`] = value
}
}
}
return defineEnv
}

/**
* Collects the `env` config value from the Next.js config.
*/
function getNextConfigEnv(config: NextConfigComplete): DefineEnv {
// Refactored code below to use for-of
const defineEnv: DefineEnv = {}
const env = config.env
for (const key in env) {
const value = env[key]
if (value) {
errorIfEnvConflicted(config, key)
defineEnv[`process.env.${key}`] = value
}
}
return defineEnv
}

/**
* Serializes the DefineEnv config so that it can be inserted into the code by Webpack/Turbopack, JSON stringifies each value.
*/
function serializeDefineEnv(defineEnv: DefineEnv): SerializedDefineEnv {
const defineEnvStringified: SerializedDefineEnv = {}
for (const key in defineEnv) {
const value = defineEnv[key]
defineEnvStringified[key] = JSON.stringify(value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
defineEnvStringified[key] = JSON.stringify(value)
defineEnvStringified[key] = value === undefined ? 'undefined' : JSON.stringify(value)

}

return defineEnvStringified
}

function getImageConfig(
config: NextConfigComplete,
dev: boolean
): { 'process.env.__NEXT_IMAGE_OPTS': Partial<NextConfigComplete['images']> } {
return {
'process.env.__NEXT_IMAGE_OPTS': {
deviceSizes: config.images.deviceSizes,
imageSizes: config.images.imageSizes,
path: config.images.path,
loader: config.images.loader,
dangerouslyAllowSVG: config.images.dangerouslyAllowSVG,
unoptimized: config?.images?.unoptimized,
...(dev
? {
// pass domains in development to allow validating on the client
domains: config.images.domains,
remotePatterns: config.images?.remotePatterns,
output: config.output,
}
: {}),
},
}
}

export function getDefineEnv({
isTurbopack,
allowedRevalidateHeaderKeys,
Expand All @@ -53,180 +142,117 @@ export function getDefineEnv({
isNodeServer,
middlewareMatchers,
previewModeId,
}: DefineEnvPluginOptions) {
return {
}: DefineEnvPluginOptions): SerializedDefineEnv {
const defineEnv: DefineEnv = {
// internal field to identify the plugin config
__NEXT_DEFINE_ENV: 'true',
__NEXT_DEFINE_ENV: true,

...Object.keys(process.env).reduce(
(prev: { [key: string]: string }, key: string) => {
if (key.startsWith('NEXT_PUBLIC_')) {
prev[`process.env.${key}`] = JSON.stringify(process.env[key]!)
}
return prev
},
{}
),
...Object.keys(config.env).reduce((acc, key) => {
errorIfEnvConflicted(config, key)

return {
...acc,
[`process.env.${key}`]: JSON.stringify(config.env[key]),
}
}, {}),
...getNextPublicEnvironmentVariables(),
...getNextConfigEnv(config),
...(!isEdgeServer
? {}
: {
EdgeRuntime: JSON.stringify(
EdgeRuntime:
/**
* Cloud providers can set this environment variable to allow users
* and library authors to have different implementations based on
* the runtime they are running with, if it's not using `edge-runtime`
*/
process.env.NEXT_EDGE_RUNTIME_PROVIDER || 'edge-runtime'
),
process.env.NEXT_EDGE_RUNTIME_PROVIDER || 'edge-runtime',
}),
'process.turbopack': JSON.stringify(isTurbopack),
'process.env.TURBOPACK': JSON.stringify(isTurbopack),
'process.turbopack': isTurbopack,
'process.env.TURBOPACK': isTurbopack,
// TODO: enforce `NODE_ENV` on `process.env`, and add a test:
'process.env.NODE_ENV': JSON.stringify(dev ? 'development' : 'production'),
'process.env.NEXT_RUNTIME': JSON.stringify(
isEdgeServer ? 'edge' : isNodeServer ? 'nodejs' : ''
),
'process.env.NEXT_MINIMAL': JSON.stringify(''),
'process.env.__NEXT_PPR': JSON.stringify(config.experimental.ppr === true),
'process.env.__NEXT_ACTIONS_DEPLOYMENT_ID': JSON.stringify(
config.experimental.useDeploymentIdServerActions
),
'process.env.NEXT_DEPLOYMENT_ID': JSON.stringify(
config.experimental.deploymentId || false
),
'process.env.__NEXT_FETCH_CACHE_KEY_PREFIX':
JSON.stringify(fetchCacheKeyPrefix),
'process.env.__NEXT_PREVIEW_MODE_ID': JSON.stringify(previewModeId),
'process.env.__NEXT_ALLOWED_REVALIDATE_HEADERS': JSON.stringify(
allowedRevalidateHeaderKeys
),
'process.env.__NEXT_MIDDLEWARE_MATCHERS': JSON.stringify(
middlewareMatchers || []
),
'process.env.__NEXT_MANUAL_CLIENT_BASE_PATH': JSON.stringify(
config.experimental.manualClientBasePath
),
'process.env.__NEXT_CLIENT_ROUTER_FILTER_ENABLED': JSON.stringify(
config.experimental.clientRouterFilter
),
'process.env.__NEXT_CLIENT_ROUTER_S_FILTER': JSON.stringify(
clientRouterFilters?.staticFilter
),
'process.env.__NEXT_CLIENT_ROUTER_D_FILTER': JSON.stringify(
clientRouterFilters?.dynamicFilter
),
'process.env.__NEXT_OPTIMISTIC_CLIENT_CACHE': JSON.stringify(
config.experimental.optimisticClientCache
),
'process.env.__NEXT_MIDDLEWARE_PREFETCH': JSON.stringify(
config.experimental.middlewarePrefetch
),
'process.env.__NEXT_CROSS_ORIGIN': JSON.stringify(config.crossOrigin),
'process.browser': JSON.stringify(isClient),
'process.env.__NEXT_TEST_MODE': JSON.stringify(
process.env.__NEXT_TEST_MODE
),
'process.env.NODE_ENV': dev ? 'development' : 'production',
'process.env.NEXT_RUNTIME': isEdgeServer
? 'edge'
: isNodeServer
? 'nodejs'
: '',
'process.env.NEXT_MINIMAL': '',
'process.env.__NEXT_PPR': config.experimental.ppr === true,
'process.env.__NEXT_ACTIONS_DEPLOYMENT_ID':
config.experimental.useDeploymentIdServerActions,
'process.env.NEXT_DEPLOYMENT_ID': config.experimental.deploymentId || false,
'process.env.__NEXT_FETCH_CACHE_KEY_PREFIX': fetchCacheKeyPrefix,
'process.env.__NEXT_PREVIEW_MODE_ID': previewModeId,
'process.env.__NEXT_ALLOWED_REVALIDATE_HEADERS':
allowedRevalidateHeaderKeys,
'process.env.__NEXT_MIDDLEWARE_MATCHERS': middlewareMatchers || [],
'process.env.__NEXT_MANUAL_CLIENT_BASE_PATH':
config.experimental.manualClientBasePath,
'process.env.__NEXT_CLIENT_ROUTER_FILTER_ENABLED':
config.experimental.clientRouterFilter,
'process.env.__NEXT_CLIENT_ROUTER_S_FILTER':
clientRouterFilters?.staticFilter,
'process.env.__NEXT_CLIENT_ROUTER_D_FILTER':
clientRouterFilters?.dynamicFilter,
'process.env.__NEXT_OPTIMISTIC_CLIENT_CACHE':
config.experimental.optimisticClientCache,
'process.env.__NEXT_MIDDLEWARE_PREFETCH':
config.experimental.middlewarePrefetch,
'process.env.__NEXT_CROSS_ORIGIN': config.crossOrigin,
'process.browser': isClient,
'process.env.__NEXT_TEST_MODE': process.env.__NEXT_TEST_MODE,
// This is used in client/dev-error-overlay/hot-dev-client.js to replace the dist directory
...(dev && (isClient || isEdgeServer)
? {
'process.env.__NEXT_DIST_DIR': JSON.stringify(distDir),
'process.env.__NEXT_DIST_DIR': distDir,
}
: {}),
'process.env.__NEXT_TRAILING_SLASH': JSON.stringify(config.trailingSlash),
'process.env.__NEXT_BUILD_INDICATOR': JSON.stringify(
config.devIndicators.buildActivity
),
'process.env.__NEXT_BUILD_INDICATOR_POSITION': JSON.stringify(
config.devIndicators.buildActivityPosition
),
'process.env.__NEXT_STRICT_MODE': JSON.stringify(
config.reactStrictMode === null ? false : config.reactStrictMode
),
'process.env.__NEXT_STRICT_MODE_APP': JSON.stringify(
'process.env.__NEXT_TRAILING_SLASH': config.trailingSlash,
'process.env.__NEXT_BUILD_INDICATOR': config.devIndicators.buildActivity,
'process.env.__NEXT_BUILD_INDICATOR_POSITION':
config.devIndicators.buildActivityPosition,
'process.env.__NEXT_STRICT_MODE':
config.reactStrictMode === null ? false : config.reactStrictMode,
'process.env.__NEXT_STRICT_MODE_APP':
// When next.config.js does not have reactStrictMode it's enabled by default.
config.reactStrictMode === null ? true : config.reactStrictMode
),
'process.env.__NEXT_OPTIMIZE_FONTS': JSON.stringify(
!dev && config.optimizeFonts
),
'process.env.__NEXT_OPTIMIZE_CSS': JSON.stringify(
config.experimental.optimizeCss && !dev
),
'process.env.__NEXT_SCRIPT_WORKERS': JSON.stringify(
config.experimental.nextScriptWorkers && !dev
),
'process.env.__NEXT_SCROLL_RESTORATION': JSON.stringify(
config.experimental.scrollRestoration
),
'process.env.__NEXT_IMAGE_OPTS': JSON.stringify({
deviceSizes: config.images.deviceSizes,
imageSizes: config.images.imageSizes,
path: config.images.path,
loader: config.images.loader,
dangerouslyAllowSVG: config.images.dangerouslyAllowSVG,
unoptimized: config?.images?.unoptimized,
...(dev
? {
// pass domains in development to allow validating on the client
domains: config.images.domains,
remotePatterns: config.images?.remotePatterns,
output: config.output,
}
: {}),
}),
'process.env.__NEXT_ROUTER_BASEPATH': JSON.stringify(config.basePath),
'process.env.__NEXT_STRICT_NEXT_HEAD': JSON.stringify(
config.experimental.strictNextHead
),
'process.env.__NEXT_HAS_REWRITES': JSON.stringify(hasRewrites),
'process.env.__NEXT_CONFIG_OUTPUT': JSON.stringify(config.output),
'process.env.__NEXT_I18N_SUPPORT': JSON.stringify(!!config.i18n),
'process.env.__NEXT_I18N_DOMAINS': JSON.stringify(config.i18n?.domains),
'process.env.__NEXT_ANALYTICS_ID': JSON.stringify(config.analyticsId), // TODO: remove in the next major version
'process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE': JSON.stringify(
config.skipMiddlewareUrlNormalize
),
'process.env.__NEXT_EXTERNAL_MIDDLEWARE_REWRITE_RESOLVE': JSON.stringify(
config.experimental.externalMiddlewareRewritesResolve
),
'process.env.__NEXT_MANUAL_TRAILING_SLASH': JSON.stringify(
config.skipTrailingSlashRedirect
),
'process.env.__NEXT_HAS_WEB_VITALS_ATTRIBUTION': JSON.stringify(
config.reactStrictMode === null ? true : config.reactStrictMode,
'process.env.__NEXT_OPTIMIZE_FONTS': !dev && config.optimizeFonts,
'process.env.__NEXT_OPTIMIZE_CSS': config.experimental.optimizeCss && !dev,
'process.env.__NEXT_SCRIPT_WORKERS':
config.experimental.nextScriptWorkers && !dev,
'process.env.__NEXT_SCROLL_RESTORATION':
config.experimental.scrollRestoration,
...getImageConfig(config, dev),
'process.env.__NEXT_ROUTER_BASEPATH': config.basePath,
'process.env.__NEXT_STRICT_NEXT_HEAD': config.experimental.strictNextHead,
'process.env.__NEXT_HAS_REWRITES': hasRewrites,
'process.env.__NEXT_CONFIG_OUTPUT': config.output,
'process.env.__NEXT_I18N_SUPPORT': !!config.i18n,
'process.env.__NEXT_I18N_DOMAINS': config.i18n?.domains,
'process.env.__NEXT_ANALYTICS_ID': config.analyticsId, // TODO: remove in the next major version
'process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE':
config.skipMiddlewareUrlNormalize,
'process.env.__NEXT_EXTERNAL_MIDDLEWARE_REWRITE_RESOLVE':
config.experimental.externalMiddlewareRewritesResolve,
'process.env.__NEXT_MANUAL_TRAILING_SLASH':
config.skipTrailingSlashRedirect,
'process.env.__NEXT_HAS_WEB_VITALS_ATTRIBUTION':
config.experimental.webVitalsAttribution &&
config.experimental.webVitalsAttribution.length > 0
),
'process.env.__NEXT_WEB_VITALS_ATTRIBUTION': JSON.stringify(
config.experimental.webVitalsAttribution
),
'process.env.__NEXT_LINK_NO_TOUCH_START': JSON.stringify(
config.experimental.linkNoTouchStart
),
'process.env.__NEXT_ASSET_PREFIX': JSON.stringify(config.assetPrefix),
config.experimental.webVitalsAttribution.length > 0,
'process.env.__NEXT_WEB_VITALS_ATTRIBUTION':
config.experimental.webVitalsAttribution,
'process.env.__NEXT_LINK_NO_TOUCH_START':
config.experimental.linkNoTouchStart,
'process.env.__NEXT_ASSET_PREFIX': config.assetPrefix,
...(isNodeOrEdgeCompilation
? {
// Fix bad-actors in the npm ecosystem (e.g. `node-formidable`)
// This is typically found in unmaintained modules from the
// pre-webpack era (common in server-side code)
'global.GENTLY': JSON.stringify(false),
'global.GENTLY': false,
}
: undefined),
...(isNodeOrEdgeCompilation
? {
'process.env.__NEXT_EXPERIMENTAL_REACT': JSON.stringify(
needsExperimentalReact(config)
),
'process.env.__NEXT_EXPERIMENTAL_REACT':
needsExperimentalReact(config),
}
: undefined),
}
return serializeDefineEnv(defineEnv)
}

export function getDefineEnvPlugin(options: DefineEnvPluginOptions) {
Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/server/config-shared.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ export type NextConfigComplete = Required<NextConfig> & {
configFileName: string
}

export type I18NDomains = DomainLocale[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably no need for this type. You can just re-export DomainLocale and wherever I18NDomains is needed, you do DomainLocale[] instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer having this explicit type instead of re-exporting DomainLocale as then you end up with scattered DomainLocale[] which specifically refers to nextConfig.i18n.domains. Could have also used NextConfigComplete['i18n']['domains]' with Omit<> but personally prefer adding additional types over trying to coerce the type into only what you need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but this makes the type a bit less reusable. The name probably could have been kept as DomainLocales instead. I18NDomains is made up specifically for the object below. Which I think was the intention, so maybe it's fine. 👍


export interface I18NConfig {
defaultLocale: string
domains?: DomainLocale[]
domains?: I18NDomains
localeDetection?: false
locales: string[]
}
Expand Down