Skip to content

Commit acd6d58

Browse files
author
Luca Forstner
authored
fix(nextjs): Don't modify require calls in wrapping loader (#6979)
1 parent 66ebf2f commit acd6d58

File tree

4 files changed

+56
-1
lines changed

4 files changed

+56
-1
lines changed

packages/nextjs/src/config/loaders/wrappingLoader.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,15 @@ async function wrapUserCode(
153153
// People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to
154154
// handle that correctly so we let a plugin to take care of bundling cjs exports for us.
155155
commonjs({
156-
transformMixedEsModules: true,
157156
sourceMap: true,
157+
strictRequires: true, // Don't hoist require statements that users may define
158+
ignoreDynamicRequires: true, // Don't break dynamic requires and things like Webpack's `require.context`
159+
ignore() {
160+
// We want basically only want to use this plugin for handling the case where users export their handlers with module.exports.
161+
// This plugin would also be able to convert any `require` into something esm compatible but webpack does that anyways so we just skip that part of the plugin.
162+
// (Also, modifying require may break user code)
163+
return true;
164+
},
158165
}),
159166
],
160167

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { NextApiResponse, NextApiRequest } from 'next';
2+
3+
if (process.env.NEXT_PUBLIC_SOME_FALSE_ENV_VAR === 'enabled') {
4+
require('../../test/server/utils/throw'); // Should not throw unless the hoisting in the wrapping loader is messed up!
5+
}
6+
7+
const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
8+
require('@sentry/nextjs').captureException; // Should not throw unless the wrapping loader messes up cjs imports
9+
// @ts-ignore
10+
require.context('.'); // This is a webpack utility call. Should not throw unless the wrapping loader messes it up by mangling.
11+
res.status(200).json({ success: true });
12+
};
13+
14+
module.exports = handler;

packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,37 @@ describe('CommonJS API Endpoints', () => {
6666
success: true,
6767
});
6868
});
69+
70+
it('should not mess up require statements', async () => {
71+
const env = await NextTestEnv.init();
72+
const route = '/api/requireTest';
73+
const url = `${env.url}${route}`;
74+
75+
const wrappedEnvelope = await env.getEnvelopeRequest({
76+
url,
77+
envelopeType: 'transaction',
78+
endServer: false,
79+
});
80+
81+
expect(wrappedEnvelope[2]).toMatchObject({
82+
contexts: {
83+
trace: {
84+
op: 'http.server',
85+
status: 'ok',
86+
tags: { 'http.status_code': '200' },
87+
},
88+
},
89+
transaction: `GET ${route}`,
90+
type: 'transaction',
91+
request: {
92+
url,
93+
},
94+
});
95+
96+
const response = await env.getAPIResponse(url);
97+
98+
expect(response).toMatchObject({
99+
success: true,
100+
});
101+
});
69102
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
throw new Error('I am throwing');

0 commit comments

Comments
 (0)