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(nextjs): Fix conflict with other libraries modifying webpack entry property #3703

Merged
merged 1 commit into from Jun 17, 2021

Conversation

lobsterkatie
Copy link
Member

In order to include the user's sentry.client.config.js code (which is where Sentry.init() is called) in the browser bundle, we modify the entry webpack configuration so that the main entry point includes that file in addition to the core nextjs browser code.

At some point in the past, that same entry point was called main.js, and next maintains code to preserve backwards compatibility with anything which still uses that name for it. In that code, anything in main.js is copied over to main before main.js is deleted. Should be fine, right?

It would be, except that instead of copying the main.js stuff into the current value of main (which by that point includes sentry.client.config.js), it instead uses the original main value (which doesn't include our code). Luckily for us, this only happens if there's anything actually in main.js, so as long as we can guarantee that it's empty, we're good.

In most cases there's nothing to do, because it's empty by default. But currently, if another library has added something to it, it triggers next's copying over and the Sentry code is lost. This fixes that by doing the copying ourselves, and then emptying main.js before next can get ahold of it.

Fixing this also exposed a mistake in our types, so that's fixed here as well.

Fixes #3538.

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

👍

@github-actions
Copy link
Contributor

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.01 KB (0%)
@sentry/browser - Webpack 21.89 KB (0%)
@sentry/react - Webpack 21.92 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.4 KB (+0.01% 🔺)

@lobsterkatie lobsterkatie merged commit 67b7685 into master Jun 17, 2021
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-handle-non-empty-mainjs-entry branch June 17, 2021 21:22
@nihalanisumit
Copy link

Thanks for fixing it, will try if it works now :)

@nihalanisumit
Copy link

const withPlugins = require('next-compose-plugins')
const { withSentryConfig } = require('@sentry/nextjs')
const withPWA = require('next-pwa')
const runtimeCaching = require('next-pwa/cache')
const withSvgr = require('next-svgr')

const SentryWebpackPluginOptions = {
  // For all available options, see:
  // https://github.com/getsentry/sentry-webpack-plugin#options.
}

const PWAPluginOptions = {
  pwa: {
    dest: 'public',
    runtimeCaching,
    // disable: process.env.NODE_ENV !== 'production',
  },
}

module.exports = withPlugins([withSvgr, [withPWA, PWAPluginOptions], [withSentryConfig, SentryWebpackPluginOptions]])

I tried using the latest build 6.7.1 and sentry doesn't work with withPlugins like this above and sentry still didn't work

then i tried doing this

const { withSentryConfig } = require('@sentry/nextjs')
const withPWA = require('next-pwa')
const runtimeCaching = require('next-pwa/cache')

const SentryWebpackPluginOptions = {
  // For all available options, see:
  // https://github.com/getsentry/sentry-webpack-plugin#options.
}

const moduleExports = withPWA({
  pwa: {
    dest: 'public',
    runtimeCaching,
  },
  webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      issuer: {
        test: /\.(js|ts)x?$/,
      },
      use: ['@svgr/webpack'],
    })

    return config
  },
})

module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions)

sentry is still not working

I want to use next-pwa, SVG, and sentry together.
Can you suggest a solution?

@AbhiPrasad
Copy link
Member

Hey @nihalanisumit, we have not cut a release with this change yet. The next version of the SDK will have these changes.

@anguskeatinge
Copy link

Solid one @lobsterkatie, keen for this release.

@lobsterkatie
Copy link
Member Author

@nihalanisumit, @anguskeatinge - this is out now, in 6.7.2 and higher.

@nihalanisumit
Copy link

Amazing it works as expected!!!

@lobsterkatie
Copy link
Member Author

Just in time for us to change approaches and make the entire thing a moot point, LOL. That change should be released sometime next week.

Glad it's working for you in the meantime, though! 🙂

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.

Next.js plugin doesn't work with next-pwa plugin #213
4 participants