Skip to content

fix(nextjs): Don't modify require calls in wrapping loader #6979

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

Merged
merged 4 commits into from
Jan 31, 2023
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
9 changes: 8 additions & 1 deletion packages/nextjs/src/config/loaders/wrappingLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,15 @@ async function wrapUserCode(
// People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to
// handle that correctly so we let a plugin to take care of bundling cjs exports for us.
commonjs({
transformMixedEsModules: true,
sourceMap: true,
strictRequires: true, // Don't hoist require statements that users may define
ignoreDynamicRequires: true, // Don't break dynamic requires and things like Webpack's `require.context`
ignore() {
// We want basically only want to use this plugin for handling the case where users export their handlers with module.exports.
// 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.
// (Also, modifying require may break user code)
return true;
},
}),
],

Expand Down
14 changes: 14 additions & 0 deletions packages/nextjs/test/integration/pages/api/requireTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { NextApiResponse, NextApiRequest } from 'next';

if (process.env.NEXT_PUBLIC_SOME_FALSE_ENV_VAR === 'enabled') {
require('../../test/server/utils/throw'); // Should not throw unless the hoisting in the wrapping loader is messed up!
}

const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
require('@sentry/nextjs').captureException; // Should not throw unless the wrapping loader messes up cjs imports
// @ts-ignore
require.context('.'); // This is a webpack utility call. Should not throw unless the wrapping loader messes it up by mangling.
res.status(200).json({ success: true });
};

module.exports = handler;
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,37 @@ describe('CommonJS API Endpoints', () => {
success: true,
});
});

it('should not mess up require statements', async () => {
const env = await NextTestEnv.init();
const route = '/api/requireTest';
const url = `${env.url}${route}`;

const wrappedEnvelope = await env.getEnvelopeRequest({
url,
envelopeType: 'transaction',
endServer: false,
});

expect(wrappedEnvelope[2]).toMatchObject({
contexts: {
trace: {
op: 'http.server',
status: 'ok',
tags: { 'http.status_code': '200' },
},
},
transaction: `GET ${route}`,
type: 'transaction',
request: {
url,
},
});

const response = await env.getAPIResponse(url);

expect(response).toMatchObject({
success: true,
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('I am throwing');