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

fix(astro): Configure default sourcemap assets glob for Vercel adapter #9665

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Nov 27, 2023

This PR adds support for uploading sourcemaps when using the @astrojs/vercel/* adapters.

It seems that the adapter updates the config.outdir value too late for our previous detection logic to
correctly detect the .vercel output directory. Furthermore, the adapter for some reason first saves server
files to /dist only to copy it later to /.vercel/functions. This copy command seems to happen too
late for when our sourcemaps plugin runs.

For these two reasons, we try to detect the used adapter (love that this is much simpler in Astro than
Sveltekit) and adjust the assets glob to search for files in both, dist and .vercel.

Tested this for client side (island) and server-side errors with a test app deployed to vercel (serverless, not
edge) and it seems to work well.

Note: I'll open a separate PR for users to specify a custom assets glob for more advanced setups.

closes #9591

@Lms24 Lms24 self-assigned this Nov 27, 2023
@Lms24 Lms24 force-pushed the lms/fix-astro-vercel-sourcemaps branch from c580cd2 to 0303ac4 Compare November 27, 2023 16:51
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 65.86 KB (-0.24% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 59.46 KB (+5.79% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.11 KB (-0.26% 🔽)
@sentry/browser - Webpack (gzipped) 21.38 KB (-0.12% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 62.71 KB (+0.11% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 29.52 KB (+0.19% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 21.6 KB (+0.2% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 197.46 KB (+0.1% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 89.32 KB (+0.2% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 64.29 KB (+0.27% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 32.21 KB (+0.29% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 66.24 KB (-0.28% 🔽)
@sentry/react - Webpack (gzipped) 21.42 KB (-0.12% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 82.96 KB (-0.25% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 48.21 KB (-0.23% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 16.19 KB (-0.02% 🔽)

// only copied over to <root>/.vercel. This seems to happen too late though.
// So we glob on both of these directories.
// Another case of "it ain't pretty but it works":(
if (config.adapter && config.adapter.name?.startsWith('@astrojs/vercel')) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (config.adapter && config.adapter.name?.startsWith('@astrojs/vercel')) {
if (config.adapter?.name?.startsWith('@astrojs/vercel')) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, good catch, just saw it too late. Will sneak your suggestion into #9668

Copy link
Member Author

Choose a reason for hiding this comment

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

@Lms24 Lms24 merged commit 85a494a into develop Nov 28, 2023
49 checks passed
@Lms24 Lms24 deleted the lms/fix-astro-vercel-sourcemaps branch November 28, 2023 08:00
@Lms24 Lms24 changed the title fix(astro): Configure sourcemap assets directory for Vercel adapter fix(astro): Configure default sourcemap assets glob for Vercel adapter Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Astro integration upload source map warning
3 participants