From 62346050c621c6c84f792d66dc73a67ebd9b8fe0 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Mon, 10 Jan 2022 13:26:50 +0200 Subject: [PATCH 1/3] Allow dependencies to use environment variables in middlewares After discussing with @sokra, seems that the proposed solution is split in two: * We need to make sure that the `process` polyfill uses `global.process` if available. This is because middlewares are bundled using `browser` target and therefore `process.env.MY_ENV` gets shimmed into `require('process').env.MY_ENV`. * Allow `process.env` to be statically analyzed for dependencies so they will be exported to the manifest. Related issues: * should fix #33043. --- packages/next/build/polyfills/process.js | 5 +++++ packages/next/build/webpack-config.ts | 2 +- packages/next/build/webpack/plugins/middleware-plugin.ts | 4 +--- 3 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 packages/next/build/polyfills/process.js diff --git a/packages/next/build/polyfills/process.js b/packages/next/build/polyfills/process.js new file mode 100644 index 0000000000000..fdd652b372d2e --- /dev/null +++ b/packages/next/build/polyfills/process.js @@ -0,0 +1,5 @@ +if (global.process) { + module.exports = global.process +} else { + module.exports = require('../../compiled/process') +} diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index bbac49486d94a..d60e3864dca69 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -635,7 +635,7 @@ export default async function getBaseWebpackConfig( os: require.resolve('next/dist/compiled/os-browserify'), path: require.resolve('next/dist/compiled/path-browserify'), punycode: require.resolve('next/dist/compiled/punycode'), - process: require.resolve('next/dist/compiled/process'), + process: require.resolve('./polyfills/process'), // Handled in separate alias querystring: require.resolve('next/dist/compiled/querystring-es3'), // TODO: investigate ncc'ing stream-browserify diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index 8c1ac1354544f..d4757ef90a40d 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -291,8 +291,6 @@ export default class MiddlewarePlugin { .tap(PLUGIN_NAME, ignore) const memberChainHandler = (_expr: any, members: string[]) => { - if (!isMiddlewareModule()) return - if (members.length >= 2 && members[0] === 'env') { const envName = members[1] const { buildInfo } = parser.state.module @@ -301,7 +299,7 @@ export default class MiddlewarePlugin { } buildInfo.nextUsedEnvVars.add(envName) - return true + if (isMiddlewareModule()) return true } } From 9988e92905d84535ca061cc1c695ebf84cdaaa1b Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Mon, 10 Jan 2022 14:01:35 +0200 Subject: [PATCH 2/3] Add a test that verifies the changes --- .../index.test.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts diff --git a/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts b/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts new file mode 100644 index 0000000000000..356aa4174b345 --- /dev/null +++ b/test/production/dependencies-can-use-env-vars-in-middlewares/index.test.ts @@ -0,0 +1,73 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { renderViaHTTP } from 'next-test-utils' +import { readJson } from 'fs-extra' +import path from 'path' + +describe('dependencies can use env vars in middlewares', () => { + let next: NextInstance + + beforeAll(() => { + process.env.MY_CUSTOM_PACKAGE_ENV_VAR = 'my-custom-package-env-var' + process.env.ENV_VAR_USED_IN_MIDDLEWARE = 'env-var-used-in-middleware' + }) + + beforeAll(async () => { + next = await createNext({ + files: { + // A 3rd party dependency + 'node_modules/my-custom-package/package.json': JSON.stringify({ + name: 'my-custom-package', + version: '1.0.0', + browser: 'index.js', + }), + 'node_modules/my-custom-package/index.js': ` + module.exports = () => process.env.MY_CUSTOM_PACKAGE_ENV_VAR; + `, + + // The actual middleware code + 'pages/api/_middleware.js': ` + import customPackage from 'my-custom-package'; + export default function middleware(_req) { + return new Response(JSON.stringify({ + string: "a constant string", + hello: process.env.ENV_VAR_USED_IN_MIDDLEWARE, + customPackage: customPackage(), + }), { + headers: { + 'Content-Type': 'application/json' + } + }) + } + `, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('parses the env vars correctly', async () => { + const testDir = next.testDir + const manifestPath = path.join( + testDir, + '.next/server/middleware-manifest.json' + ) + const manifest = await readJson(manifestPath) + const envVars = manifest?.middleware?.['/api']?.env + + expect(envVars).toHaveLength(2) + expect(envVars).toContain('ENV_VAR_USED_IN_MIDDLEWARE') + expect(envVars).toContain('MY_CUSTOM_PACKAGE_ENV_VAR') + }) + + it('uses the environment variables', async () => { + const html = await renderViaHTTP(next.url, '/api') + expect(html).toContain( + JSON.stringify({ + string: 'a constant string', + hello: 'env-var-used-in-middleware', + customPackage: 'my-custom-package-env-var', + }) + ) + }) +}) From f2cc4631de9147ca501b49416720936e7e80e210 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Mon, 10 Jan 2022 16:10:34 +0200 Subject: [PATCH 3/3] Update packages/next/build/polyfills/process.js Co-authored-by: Tobias Koppers --- packages/next/build/polyfills/process.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/next/build/polyfills/process.js b/packages/next/build/polyfills/process.js index fdd652b372d2e..054c87b910886 100644 --- a/packages/next/build/polyfills/process.js +++ b/packages/next/build/polyfills/process.js @@ -1,5 +1 @@ -if (global.process) { - module.exports = global.process -} else { - module.exports = require('../../compiled/process') -} +module.exports = global.process || require('../../compiled/process')