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): Fix import path when using external init files with default path #10214

Merged
merged 3 commits into from Jan 17, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jan 17, 2024

This PR fixes a pretty obvious bug in the Astro SDK where specifying an external sentry.(client|server).config.js in the default location file would cause a build error. Looks like this slipped through at some point and we didn't test it properly.

Previously, we checked for a supported file extension and instead of adding an import statement for the path to the file, we'd only add the file extension. So

import 'mjs';
// instead of 
import 'path/to/sentry.client.config.mjs'

This PR fixes the filename lookup function and adds a test to cover regressions.

cc @HazAT

Copy link
Contributor

github-actions bot commented Jan 17, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.3 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.61 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.26 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.63 KB (+0.05% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.13 KB (0%)
@sentry/browser - Webpack (gzipped) 22.48 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 74.97 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 66.6 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 32.43 KB (+0.04% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.19 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 209.71 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 97.77 KB (+0.06% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 72.34 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 35.47 KB (+0.07% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69 KB (+0.02% 🔺)
@sentry/react - Webpack (gzipped) 22.52 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.62 KB (+0.02% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 49.75 KB (+0.04% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 17.11 KB (0%)

Copy link
Member Author

@Lms24 Lms24 Jan 17, 2024

Choose a reason for hiding this comment

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

Had to add a second test file here to avoid globally mocking the fs.existsSync implementation for the other tests which relied on its default implementation.

Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

🙏

@Lms24 Lms24 merged commit 70d1cbb into develop Jan 17, 2024
54 checks passed
@Lms24 Lms24 deleted the lms/fix-astro-default-path-init-file branch January 17, 2024 14:07
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.

None yet

2 participants