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(config): enhance source map handling and debugging workflow #342

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

amaringonz
Copy link

This pull request introduces improvements to the Next.js configuration for handling source maps and debugging.

Changes

  • Source Map Security:

    • Added hidden-source-map to the Webpack configuration for client builds, ensuring source maps are available for debugging while staying hidden in production environments.
  • Comments and Documentation:

    • Enhanced comments in the sourcemaps configuration to provide better clarity and guide future maintainers on the impact of this setting.
  • FIXME for Source Map Retention:

    • Included a FIXME to evaluate whether source maps should be retained locally after being uploaded to Sentry, ensuring alignment with debugging workflows.

Rationale

  • Improves production security by hiding source maps from public exposure.
  • Provides clear documentation for maintainers to understand the purpose and implications of source map handling settings.
  • Ensures the debugging workflow is adaptable for both development and production environments.

- Added `hidden-source-map` in Webpack config for client builds to secure debugging data.
- Enhanced comments in `sourcemaps` configuration for clarity and future maintainability.
- Included `FIXME` to confirm source map retention policy post-upload to Sentry.
Copy link

vercel bot commented Jan 1, 2025

@amaringonz is attempting to deploy a commit to the Ixartz's projects Team on Vercel.

A member of the Team first needs to authorize it.

@ixartz
Copy link
Owner

ixartz commented Jan 1, 2025

@amaringonz thank you so much for your PR. Is it possible to share with me the documentation you use to make these changes? It's very hard for me to understand the changes and the result.

The current configuration comes directly from Sentry using the CLI. It should be the default configuration provided by Sentry team.

@amaringonz
Copy link
Author

Thank you so much for reviewing my PR! Here’s the documentation and reasoning behind the changes I made:

1. Removal of hideSourceMaps: true

This change was based on the deprecation notice from Sentry's SDK documentation. Specifically:

  • The option hideSourceMaps: true was used to set the Webpack devtool option to hidden-source-map, which stripped the sourceMappingURL from the bottom of built JavaScript files.
  • However, this behavior is now deprecated, as the Sentry SDK automatically emits client bundles without the sourceMappingURL by default.

Relevant Documentation:

2. Addition of sourcemaps.deleteSourcemapsAfterUpload

The following warning message appears when running the build command with the Sentry SDK:

[@sentry/nextjs] The Sentry SDK has enabled source map generation for your Next.js app. If you don't want to serve Source Maps to your users, either set the `sourcemaps.deleteSourcemapsAfterUpload` option to true, or manually delete the source maps after the build. In future Sentry SDK versions `sourcemaps.deleteSourcemapsAfterUpload` will default to `true`. If you do not want to generate and upload sourcemaps, set the `sourcemaps.disable` option in `withSentryConfig()`.

To address this, I added the following configuration:

sourcemaps: {
  deleteSourcemapsAfterUpload: true,
},

This ensures:

  • The source maps are uploaded to Sentry during the build process.
  • The local source maps are deleted post-upload, avoiding the risk of them being exposed to end-users.

Relevant Documentation:


If there’s anything else that’s unclear, feel free to reach out, and I’ll be happy to clarify further!

@ixartz
Copy link
Owner

ixartz commented Jan 2, 2025

Thank you so much for your detailed response, I'll take a deep look at it, thank you for your patience.

deleteSourcemapsAfterUpload looks good to me but the part with webpack:

webpack: (config, options) => {
         if (!options.dev) {
           config.devtool = options.isServer ? false : 'hidden-source-map';
         }


         return config;
       },

Not really a huge fan making custom changes. It would be better to have Next.js config or Sentry config.

@amaringonz
Copy link
Author

Thank you for your feedback and for taking the time to review the PR! I understand your concern about making custom changes to the webpack configuration.

The approach using config.devtool in the webpack function was primarily added because Next.js currently doesn’t provide a built-in way to customize the source map behavior for production builds without modifying the webpack config directly. The goal is to generate source maps only for the client-side code while preventing them from being publicly accessible.

That said, I agree it would be ideal if Next.js or Sentry provided a dedicated option for handling source maps more elegantly. Until such a feature is available, this workaround helps maintain better control over source maps in production.

If you have any alternative suggestions or a preference for a different approach, I’d be happy to explore it further. Thanks again for your time and feedback!

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.

2 participants