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

feat(nextjs): Bump minimum required Next.js version to 13.2.0 #11097

Merged
merged 20 commits into from
Mar 15, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,11 @@ The following previously deprecated API has been removed from the `@sentry/nextj
- `IS_BUILD`
- `isBuild`

#### Updated minimum compatible Next.js version to `13.2.0`

The minimum version of Next.js compatible with the Sentry Next.js SDK has been raised to `13.2.0`. Older versions may
exhibit bugs or unexpected behaviour.

#### Merging of the Sentry Webpack Plugin options and SDK Build options

With version 8 of the Sentry Next.js SDK, `withSentryConfig` will no longer accept 3 arguments. The second argument
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
shouldSkipReplayTest,
} from '../../../../../utils/replayHelpers';

sentryTest('handles empty/missing request headers', async ({ getLocalTestPath, page, browserName }) => {
// Skipping because this test is flaky
// https://github.com/getsentry/sentry-javascript/issues/11062
sentryTest.skip('handles empty/missing request headers', async ({ getLocalTestPath, page, browserName }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}
Expand Down Expand Up @@ -250,7 +252,9 @@ sentryTest('captures request headers on Request', async ({ getLocalTestPath, pag
]);
});

sentryTest('captures request headers as Headers instance', async ({ getLocalTestPath, page, browserName }) => {
// This test is flaky.
// See https://github.com/getsentry/sentry-javascript/pull/11110
sentryTest.skip('captures request headers as Headers instance', async ({ getLocalTestPath, page, browserName }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
shouldSkipReplayTest,
} from '../../../../../utils/replayHelpers';

sentryTest('captures request body size when body is sent', async ({ getLocalTestPath, page }) => {
// Skipping because this test is flaky
// https://github.com/getsentry/sentry-javascript/issues/10395
sentryTest.skip('captures request body size when body is sent', async ({ getLocalTestPath, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}
Expand Down Expand Up @@ -93,7 +95,9 @@ sentryTest('captures request body size when body is sent', async ({ getLocalTest
]);
});

sentryTest('captures request size from non-text request body', async ({ getLocalTestPath, page }) => {
// This test is flaky.
// See https://github.com/getsentry/sentry-javascript/pull/11110
sentryTest.skip('captures request size from non-text request body', async ({ getLocalTestPath, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@
"@types/resolve": "1.20.3",
"@types/webpack": "^4.41.31",
"eslint-plugin-react": "^7.31.11",
"next": "10.1.3"
"next": "13.2.0"
},
"peerDependencies": {
"next": "^10.0.8 || ^11.0 || ^12.0 || ^13.0 || ^14.0",
"next": "^13.2.0 || ^14.0",
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should make this 14.1.3 for vercel/next.js#61194

Suggested change
"next": "^13.2.0 || ^14.0",
"next": "^13.2.0 || ^14.1.3",

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 don't understand how that would be useful. Can you elaborate? We still have to maintain anything pre 14.1.3.

Copy link
Member

Choose a reason for hiding this comment

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

Well if we make 14.1.3 the minimum for v14, we don't have to maintain anything pre 14.1.3. It incentivizes people to use the framework version that works best for the SDK, but I can also see how it introduces friction.

"react": "16.x || 17.x || 18.x",
"webpack": ">= 4.0.0"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {
import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan } from '@sentry/react';
import type { Client, TransactionSource } from '@sentry/types';
import { browserPerformanceTimeOrigin, logger, stripUrlQueryAndFragment } from '@sentry/utils';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';

import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
import RouterImport from 'next/router';

// next/router v10 is CJS
Expand All @@ -27,7 +28,7 @@ const globalObject = WINDOW as typeof WINDOW & {
/**
* Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app.
*/
interface SentryEnhancedNextData extends NextData {
interface SentryEnhancedNextData extends NEXT_DATA {
props: {
pageProps?: {
_sentryTraceData?: string; // trace parent info, if injected by a data-fetcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ export function wrapGetServerSidePropsWithSentry(
const activeSpan = getActiveSpan();
const requestTransaction = getSpanFromRequest(req) ?? (activeSpan ? getRootSpan(activeSpan) : undefined);
if (requestTransaction) {
serverSideProps.props._sentryTraceData = spanToTraceHeader(requestTransaction);
(serverSideProps.props as Record<string, unknown>)._sentryTraceData = spanToTraceHeader(requestTransaction);

const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestTransaction);
serverSideProps.props._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
(serverSideProps.props as Record<string, unknown>)._sentryBaggage =
dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/test/integration/next-env.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/// <reference types="next" />
/// <reference types="next/image-types/global" />
/// <reference types="next/navigation-types/compat/navigation" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
14 changes: 0 additions & 14 deletions packages/nextjs/test/integration/next10.config.template

This file was deleted.

15 changes: 0 additions & 15 deletions packages/nextjs/test/integration/next11.config.template

This file was deleted.

13 changes: 0 additions & 13 deletions packages/nextjs/test/integration/next12.config.template

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ const moduleExports = {
eslint: {
ignoreDuringBuilds: true,
},
experimental: {
appDir: Number(process.env.NODE_MAJOR) >= 16, // experimental.appDir requires Node v16.8.0 or later.
},
pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'page.tsx'],
};

Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/test/integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
"dependencies": {
"@sentry/nextjs": "file:../../",
"next": "latest",
"react": "^17.0.1",
"react-dom": "^17.0.1"
"react": "^18.2.0",
"react-dom": "^18.2.0"
},
"devDependencies": {
"@types/node": "^15.3.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { WINDOW } from '@sentry/react';
import type { Client } from '@sentry/types';
import { JSDOM } from 'jsdom';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import type { NEXT_DATA } from 'next/dist/shared/lib/utils';
import Router from 'next/router';

import {
Expand Down Expand Up @@ -60,7 +60,7 @@ describe('pagesRouterInstrumentPageLoad', () => {
navigatableRoutes?: string[];
hasNextData: boolean;
}) {
const nextDataContent: NextData = {
const nextDataContent: NEXT_DATA = {
props: pageProperties.props,
page: pageProperties.route,
query: pageProperties.query,
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('pagesRouterInstrumentNavigation', () => {
navigatableRoutes?: string[];
hasNextData: boolean;
}) {
const nextDataContent: NextData = {
const nextDataContent: NEXT_DATA = {
props: pageProperties.props,
page: pageProperties.route,
query: pageProperties.query,
Expand Down
110 changes: 33 additions & 77 deletions packages/nextjs/test/run-integration-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ echo "Running integration tests on Node $NODE_VERSION"
# make a backup of our config file so we can restore it when we're done
mv next.config.js next.config.js.bak

for NEXTJS_VERSION in 10 11 12 13; do
for NEXTJS_VERSION in 13; do
for USE_APPDIR in true false; do
if ([ "$NEXTJS_VERSION" -lt "13" ] || [ "$NODE_MAJOR" -lt "16" ]) && [ "$USE_APPDIR" == true ]; then
# App dir doesn not work on Next.js < 13 or Node.js < 16
if ([ "$NODE_MAJOR" -lt "16" ]) && [ "$USE_APPDIR" == true ]; then
# App dir doesn not work on Node.js < 16
AbhiPrasad marked this conversation as resolved.
Show resolved Hide resolved
continue
fi

Expand Down Expand Up @@ -64,11 +64,6 @@ for NEXTJS_VERSION in 10 11 12 13; do
sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
fi

# Next.js v13 requires React 18.2.0
if [ "$NEXTJS_VERSION" -eq "13" ]; then
npm i --save react@18.2.0 react-dom@18.2.0
fi

# Yarn install randomly started failing because it couldn't find some cache so for now we need to run these two commands which seem to fix it.
# It was pretty much this issue: https://github.com/yarnpkg/yarn/issues/5275
rm -rf node_modules
Expand All @@ -82,84 +77,45 @@ for NEXTJS_VERSION in 10 11 12 13; do
linkcli && linkplugin
mv -f package.json.bak package.json 2>/dev/null || true

SHOULD_RUN_WEBPACK_5=(true)
# Only run Webpack 4 tests for Next 10 and Next 11
if [ "$NEXTJS_VERSION" -eq "10" ] || [ "$NEXTJS_VERSION" -eq "11" ]; then
SHOULD_RUN_WEBPACK_5+=(false)
fi

for RUN_WEBPACK_5 in ${SHOULD_RUN_WEBPACK_5[*]}; do
[ "$RUN_WEBPACK_5" == true ] &&
WEBPACK_VERSION=5 ||
WEBPACK_VERSION=4

if [ "$NODE_MAJOR" -gt "17" ]; then
# Node v17+ does not work with NextJS 10 and 11 because of their legacy openssl use
# Ref: https://github.com/vercel/next.js/issues/30078
if [ "$NEXTJS_VERSION" -lt "12" ]; then
echo "[nextjs@$NEXTJS_VERSION Node $NODE_MAJOR not compatible with NextJS $NEXTJS_VERSION"
# Continues the 2nd enclosing loop, which is the outer loop that iterates over the NextJS version
continue 2
fi

# Node v18 only with Webpack 5 and above
# https://github.com/webpack/webpack/issues/14532#issuecomment-947513562
# Context: https://github.com/vercel/next.js/issues/30078#issuecomment-947338268
if [ "$WEBPACK_VERSION" -eq "4" ]; then
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Node $NODE_MAJOR not compatible with Webpack $WEBPACK_VERSION"
# Continues the 1st enclosing loop, which is the inner loop that iterates over the Webpack version
continue
fi

if [ "$NEXTJS_VERSION" -eq "13" ]; then
if [ "$USE_APPDIR" == true ]; then
cat next13.appdir.config.template > next.config.js
else
cat next13.config.template > next.config.js
fi
fi

# next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings
if [ "$NEXTJS_VERSION" -eq "10" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next10.config.template >next.config.js
elif [ "$NEXTJS_VERSION" -eq "11" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next11.config.template >next.config.js
elif [ "$NEXTJS_VERSION" -eq "12" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next12.config.template >next.config.js
elif [ "$NEXTJS_VERSION" -eq "13" ]; then
if [ "$USE_APPDIR" == true ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next13.appdir.config.template >next.config.js
else
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next13.config.template >next.config.js
fi
fi
echo "[nextjs@$NEXTJS_VERSION] Building..."
yarn build

echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Building..."
yarn build
# we keep this updated as we run the tests, so that if it's ever non-zero, we can bail
EXIT_CODE=0

# we keep this updated as we run the tests, so that if it's ever non-zero, we can bail
EXIT_CODE=0
if [ "$USE_APPDIR" == true ]; then
echo "Skipping server tests for appdir"
else
echo "[nextjs@$NEXTJS_VERSION] Running server tests with options: $args"
(cd .. && yarn test:integration:server) || EXIT_CODE=$?
fi

if [ "$USE_APPDIR" == true ]; then
echo "Skipping server tests for appdir"
else
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running server tests with options: $args"
(cd .. && yarn test:integration:server) || EXIT_CODE=$?
fi
if [ $EXIT_CODE -eq 0 ]; then
echo "[nextjs@$NEXTJS_VERSION] Server integration tests passed"
else
echo "[nextjs@$NEXTJS_VERSION] Server integration tests failed"
exit 1
fi

if [ "$NODE_MAJOR" -lt "14" ]; then
echo "[nextjs@$NEXTJS_VERSION] Skipping client tests on Node $NODE_MAJOR"
else
echo "[nextjs@$NEXTJS_VERSION] Running client tests with options: $args"
(cd .. && yarn test:integration:client) || EXIT_CODE=$?
if [ $EXIT_CODE -eq 0 ]; then
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests passed"
echo "[nextjs@$NEXTJS_VERSION] Client integration tests passed"
else
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests failed"
echo "[nextjs@$NEXTJS_VERSION] Client integration tests failed"
exit 1
fi

if [ "$NODE_MAJOR" -lt "14" ]; then
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Skipping client tests on Node $NODE_MAJOR"
else
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running client tests with options: $args"
(cd .. && yarn test:integration:client) || EXIT_CODE=$?
if [ $EXIT_CODE -eq 0 ]; then
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests passed"
else
echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests failed"
exit 1
fi
fi
done
fi
done
done
2 changes: 1 addition & 1 deletion packages/nextjs/test/types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
"test": "ts-node test.ts"
},
"dependencies": {
"next": "12.3.1"
"next": "13.2.0"
}
}
4 changes: 2 additions & 2 deletions packages/node-experimental/test/transports/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ function setupTestServer(
res.connection?.end();
});

testServer.listen(18100);
testServer.listen(18101);

return new Promise(resolve => {
testServer?.on('listening', resolve);
});
}

const TEST_SERVER_URL = 'http://localhost:18100';
const TEST_SERVER_URL = 'http://localhost:18101';

const EVENT_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
Expand Down