Skip to content

Commit 2524460

Browse files
authored
fix(v9/astro): Revert Astro v5 storing route data to globalThis (#17185)
To share the route data from build-time with the server runtime, the data was written on `globalThis`. However, this resulted in actually modifying the server config of the working directory and not the build directory (`dist` folder). This PR reverts this change for now, resulting in not having all server requests parametrized. We still do have some parametrization, as route parametrization was recently improved for Astro v4 as well (without the `'astro:routes:resolved'` hook) Fixes #17179
1 parent 38771bc commit 2524460

File tree

2 files changed

+7
-58
lines changed

2 files changed

+7
-58
lines changed

dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ test.describe('nested SSR routes (client, server, server request)', () => {
248248

249249
// Server HTTP request transaction
250250
expect(serverHTTPServerRequestTxn).toMatchObject({
251-
transaction: 'GET /api/user/[userId].json',
251+
transaction: 'GET /api/user/myUsername123.json', // fixme: should be GET /api/user/[userId].json
252252
transaction_info: { source: 'route' },
253253
contexts: {
254254
trace: {
@@ -278,13 +278,13 @@ test.describe('nested SSR routes (client, server, server request)', () => {
278278
await page.goto('/catchAll/hell0/whatever-do');
279279

280280
const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content');
281-
expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5B...path%5D');
281+
expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5Bpath%5D'); // fixme: should be %2FcatchAll%2F%5B...path%5D
282282

283283
const clientPageloadTxn = await clientPageloadTxnPromise;
284284
const serverPageRequestTxn = await serverPageRequestTxnPromise;
285285

286286
expect(clientPageloadTxn).toMatchObject({
287-
transaction: '/catchAll/[...path]',
287+
transaction: '/catchAll/[path]', // fixme: should be /catchAll/[...path]
288288
transaction_info: { source: 'route' },
289289
contexts: {
290290
trace: {
@@ -300,7 +300,7 @@ test.describe('nested SSR routes (client, server, server request)', () => {
300300
});
301301

302302
expect(serverPageRequestTxn).toMatchObject({
303-
transaction: 'GET /catchAll/[...path]',
303+
transaction: 'GET /catchAll/[path]', // fixme: should be GET /catchAll/[...path]
304304
transaction_info: { source: 'route' },
305305
contexts: {
306306
trace: {

packages/astro/src/integration/index.ts

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
1-
import { readFileSync, writeFileSync } from 'node:fs';
2-
import { consoleSandbox, debug } from '@sentry/core';
1+
import { consoleSandbox } from '@sentry/core';
32
import { sentryVitePlugin } from '@sentry/vite-plugin';
4-
import type { AstroConfig, AstroIntegration, RoutePart } from 'astro';
3+
import type { AstroConfig, AstroIntegration } from 'astro';
54
import * as fs from 'fs';
65
import * as path from 'path';
76
import { buildClientSnippet, buildSdkInitFileImportSnippet, buildServerSnippet } from './snippets';
8-
import type { IntegrationResolvedRoute, SentryOptions } from './types';
7+
import type { SentryOptions } from './types';
98

109
const PKG_NAME = '@sentry/astro';
1110

1211
export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
13-
let sentryServerInitPath: string | undefined;
14-
let didSaveRouteData = false;
15-
1612
return {
1713
name: PKG_NAME,
1814
hooks: {
@@ -138,8 +134,6 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
138134
injectScript('page-ssr', buildServerSnippet(options || {}));
139135
}
140136

141-
sentryServerInitPath = pathToServerInit;
142-
143137
// Prevent Sentry from being externalized for SSR.
144138
// Cloudflare like environments have Node.js APIs are available under `node:` prefix.
145139
// Ref: https://developers.cloudflare.com/workers/runtime-apis/nodejs/
@@ -171,36 +165,6 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
171165
});
172166
}
173167
},
174-
175-
// @ts-expect-error - This hook is available in Astro 5+
176-
'astro:routes:resolved': ({ routes }: { routes: IntegrationResolvedRoute[] }) => {
177-
if (!sentryServerInitPath || didSaveRouteData) {
178-
return;
179-
}
180-
181-
try {
182-
const serverInitContent = readFileSync(sentryServerInitPath, 'utf8');
183-
184-
const updatedServerInitContent = `${serverInitContent}\nglobalThis["__sentryRouteInfo"] = ${JSON.stringify(
185-
routes.map(route => {
186-
return {
187-
...route,
188-
patternCaseSensitive: joinRouteSegments(route.segments), // Store parametrized routes with correct casing on `globalThis` to be able to use them on the server during runtime
189-
patternRegex: route.patternRegex.source, // using `source` to be able to serialize the regex
190-
};
191-
}),
192-
null,
193-
2,
194-
)};`;
195-
196-
writeFileSync(sentryServerInitPath, updatedServerInitContent, 'utf8');
197-
198-
didSaveRouteData = true; // Prevents writing the file multiple times during runtime
199-
debug.log('Successfully added route pattern information to Sentry config file:', sentryServerInitPath);
200-
} catch (error) {
201-
debug.warn(`Failed to write to Sentry config file at ${sentryServerInitPath}:`, error);
202-
}
203-
},
204168
},
205169
};
206170
};
@@ -307,18 +271,3 @@ export function getUpdatedSourceMapSettings(
307271

308272
return { previousUserSourceMapSetting, updatedSourceMapSetting };
309273
}
310-
311-
/**
312-
* Join Astro route segments into a case-sensitive single path string.
313-
*
314-
* Astro lowercases the parametrized route. Joining segments manually is recommended to get the correct casing of the routes.
315-
* Recommendation in comment: https://github.com/withastro/astro/issues/13885#issuecomment-2934203029
316-
* Function Reference: https://github.com/joanrieu/astro-typed-links/blob/b3dc12c6fe8d672a2bc2ae2ccc57c8071bbd09fa/package/src/integration.ts#L16
317-
*/
318-
function joinRouteSegments(segments: RoutePart[][]): string {
319-
const parthArray = segments.map(segment =>
320-
segment.map(routePart => (routePart.dynamic ? `[${routePart.content}]` : routePart.content)).join(''),
321-
);
322-
323-
return `/${parthArray.join('/')}`;
324-
}

0 commit comments

Comments
 (0)