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

[Next] My webpack config is ignored since 7.22 #6339

Closed
3 tasks done
simPod opened this issue Nov 30, 2022 · 15 comments
Closed
3 tasks done

[Next] My webpack config is ignored since 7.22 #6339

simPod opened this issue Nov 30, 2022 · 15 comments

Comments

@simPod
Copy link

simPod commented Nov 30, 2022

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which package are you using?

@sentry/nextjs

SDK Version

7.22.0

Framework Version

No response

Link to Sentry event

No response

Steps to Reproduce

I believe it has something to do with

#6291

Have a custom webpack config:

/**
 * @type {import('next').NextConfig}
 */
const nextConfig = {
  ...
  sentry: {
    disableClientWebpackPlugin: true,
    disableServerWebpackPlugin: true,
  },
  webpack(config, { dev }) {

    console.log('hi');

    return config;
  },
};

Expected Result

Echoes "hi" since it's picked up by Next

Actual Result

My webpack config is ignored

@zcmk123
Copy link

zcmk123 commented Nov 30, 2022

Version 7.22.0 has lot of issues, my project can not finish build process.
At first throw sassError, i thought maybe my style file has something wrong.
Takes my whole afternoon, find out it's sentry plugin problem...

@lforst
Copy link
Member

lforst commented Nov 30, 2022

Hi, thanks for reporting this. Can you guys share your complete next configs? I have a suspicion that there are some compatibilities with other wrappers.

@simPod
Copy link
Author

simPod commented Nov 30, 2022

// @ts-check

const withOptimizedImages = require('next-optimized-images');
const { withSentryConfig } = require('@sentry/nextjs');

const plugins = [
  nextConfig =>
    withOptimizedImages({
      ...nextConfig,
      ...{
        handleImages: ['jpeg', 'png', 'webp', 'gif', 'ico'],
        optimizeImagesInDev: true,
      },
    }),
  withSentryConfig,
];

/**
 * @type {import('next').NextConfig}
 */
const nextConfig = {
  experimental: {
    transpilePackages: ['react-syntax-highlighter', 'swagger-client', 'swagger-ui-react'],
  },
  // Disable sentry plugin
  // https://github.com/getsentry/sentry-javascript/blob/115b0f3c07efa755c8f90d32cfd1783a35451eba/packages/nextjs/src/config/webpack.ts#L82-84
  sentry: {
    disableClientWebpackPlugin: true,
    disableServerWebpackPlugin: true,
  },
  webpack(config, { dev }) {

    console.log('hi');

    return config;
  },
};

module.exports = (phase, { defaultConfig }) => {
  // Next complains about unsupported options from defaultConfig https://github.com/vercel/next.js/issues/39161
  // Next complains about next-optimized-images options https://github.com/cyrilwanner/next-optimized-images/issues/285
  return plugins.reduce((acc, plugin) => plugin(acc), { ...defaultConfig, ...nextConfig });
};

@lforst
Copy link
Member

lforst commented Nov 30, 2022

withSentryConfig returns a function with the same signature as your module.exports. You need to consider that.

Quickly rewriting your config for you:

// @ts-check

const withOptimizedImages = require('next-optimized-images');
const { withSentryConfig } = require('@sentry/nextjs');

const plugins = [
  nextConfig =>
    withOptimizedImages({
      ...nextConfig,
      ...{
        handleImages: ['jpeg', 'png', 'webp', 'gif', 'ico'],
        optimizeImagesInDev: true,
      },
    })
];

/**
 * @type {import('next').NextConfig}
 */
const nextConfig = {
  experimental: {
    transpilePackages: ['react-syntax-highlighter', 'swagger-client', 'swagger-ui-react'],
  },
  // Disable sentry plugin
  // https://github.com/getsentry/sentry-javascript/blob/115b0f3c07efa755c8f90d32cfd1783a35451eba/packages/nextjs/src/config/webpack.ts#L82-84
  sentry: {
    disableClientWebpackPlugin: true,
    disableServerWebpackPlugin: true,
  },
  webpack(config, { dev }) {

    console.log('hi');

    return config;
  },
};

module.exports = (phase, { defaultConfig }) => {
  const sentrifiedConfig = withSentryConfig(nextConfig)(phase, { defaultConfig });

  // Next complains about unsupported options from defaultConfig https://github.com/vercel/next.js/issues/39161
  // Next complains about next-optimized-images options https://github.com/cyrilwanner/next-optimized-images/issues/285
  return plugins.reduce((acc, plugin) => plugin(acc), { ...defaultConfig, ...sentrifiedConfig });
};

@jesseswedlund
Copy link

jesseswedlund commented Nov 30, 2022

I'm experiencing a similar problem. I'm using svgr/webpack in my app, so the Next config I'm passing to withSentryConfig contains this code:

webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: [{ loader: '@svgr/webpack', options: { icon: true } }],
    });

    return config;
  },

I believe that withSentryConfig might be overwriting my webpack rule on line 54 of withSentryConfig.ts:

 return {
      ...userNextConfigObject,
      webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions),
    };

@lforst
Copy link
Member

lforst commented Dec 1, 2022

I'm experiencing a similar problem. I'm using svgr/webpack in my app, so the Next config I'm passing to withSentryConfig contains this code:

webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: [{ loader: '@svgr/webpack', options: { icon: true } }],
    });

    return config;
  },

I believe that withSentryConfig might be overwriting my webpack rule on line 54 of withSentryConfig.ts:

 return {
      ...userNextConfigObject,
      webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions),
    };

That is unlikely since we're pretty careful about user-provided webpack functions here: https://github.com/getsentry/sentry-javascript/blob/master/packages/nextjs/src/config/webpack.ts#LL74

Can you share your nextjs config? If anything changed, it's what is returned by withSentryConfig.

@simPod
Copy link
Author

simPod commented Dec 2, 2022

Quickly rewriting your config for you:

this works for me, thanks

@simPod
Copy link
Author

simPod commented Dec 4, 2022

I handled the case where NextConfigFunction is returned.

module.exports = (phase, { defaultConfig }) => {
  // Next complains about unsupported options from defaultConfig https://github.com/vercel/next.js/issues/39161
  // Next complains about next-optimized-images options https://github.com/cyrilwanner/next-optimized-images/issues/285
  return plugins.reduce(
    (config, plugin) => {
      const appliedPlugin = plugin(config);

      return typeof appliedPlugin === 'function'
        ? appliedPlugin(phase, { defaultConfig })
        : appliedPlugin;
    },
    { ...defaultConfig, ...nextConfig },
  );
};

@simPod simPod closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2022
@gthb
Copy link
Contributor

gthb commented Dec 10, 2022

I believe that withSentryConfig might be overwriting my webpack rule on line 54 of withSentryConfig.ts:

 return {
      ...userNextConfigObject,
      webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions),
    };

That is unlikely since we're pretty careful about user-provided webpack functions here: https://github.com/getsentry/sentry-javascript/blob/master/packages/nextjs/src/config/webpack.ts#LL74

It still seems to be what's happening for us too, blocking us from upgrading the Sentry SDK to 7.22 or newer. The Next.js build outputs a lot of this:

<w> [webpack.cache.PackFileCacheStrategy] Skipped not serializable cache item 'Compilation/modules|/home/circleci/repo/node_modules/next/dist/build/webpack/loaders/css-loader/src/index.js??ruleSet[1].rules[2].oneOf[5].use[0]!/home/circleci/repo/node_modules/next/dist/build/webpack/loaders/postcss-loader/src/index.js??ruleSet[1].rules[2].oneOf[5].use[1]!/home/circleci/repo/node_modules/next/dist/build/webpack/loaders/resolve-url-loader/index.js??ruleSet[1].rules[2].oneOf[5].use[2]!/home/circleci/repo/node_modules/next/dist/compiled/sass-loader/cjs.js??ruleSet[1].rules[2].oneOf[5].use[3]!/home/circleci/repo/src/pages/new/new.module.scss': No serializer registered for SassError
<w> while serializing webpack/lib/cache/PackFileCacheStrategy.PackContentItems -> webpack/lib/NormalModule -> webpack/lib/ModuleBuildError -> SassError

followed by a lot of this:

info  - Creating an optimized production build  
Failed to compile.

./src/components/AddDrive/AddDrive.module.scss
SassError: Can't find stylesheet to import.
  ╷
1 │ @import "styles";
  │         ^^^^^^^^
  ╵
  src/components/AddDrive/AddDrive.module.scss 1:9  root stylesheet

The workaround that @simPod said worked for them (“Quickly rewriting your config for you”) might well work, but it makes the configuration quite different from the documented way to configure Sentry into next.config.js: https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#extend-nextjs-configuration — which is what we have done, and which breaks with version 7.22.

So I think this breaking change still needs some attention.

@lforst
Copy link
Member

lforst commented Dec 12, 2022

@gthb Can you share your entire next.config.js? I'd like to understand a little better what is causing the error in your case.

So I think this breaking change still needs some attention.

While I agree that this change still needs some attention, since we get a lot of similar issues at the moment, I disagree that this is breaking. Ideally we either adjust the docs, print some error in case the returned function is used as an object config or both.

@gthb
Copy link
Contributor

gthb commented Dec 12, 2022

@gthb Can you share your entire next.config.js? I'd like to understand a little better what is causing the error in your case.

Sure — I have just emailed it to you privately (to the email address in your Github profile), just redacting a few constants with credentials in them.

I disagree that this is breaking.

Oh, by “breaking” I just mean that there exist apparently legitimate Sentry-Next.js setups which work with 7.21.0 but break when upgrading to 7.22.0, so they need to adapt in some way other than just bumping the version. I don't mean to imply anything about severity or importance. :)

@lforst
Copy link
Member

lforst commented Dec 12, 2022

I disagree that this is breaking.

Oh, by “breaking” I just mean that there exist apparently legitimate Sentry-Next.js setups which work with 7.21.0 but break when upgrading to 7.22.0, so they need to adapt in some way other than just bumping the version. I don't mean to imply anything about severity or importance. :)

As a library maintainer, I am well aware of what breaking means. :) The change we did wasn't breaking in any of the word's meanings. All configurations that broke with the update weren't valid configurations in the first place. The docs instruct to put the Sentry plugin last - if this is not the case, it is not a problem with the library but rather misconfiguration. I am aware that you can't always put a plugin last (for example if two plugins tell you to do that) - actually the Sentry plugin doesn't even have to come last, we just tell you to do that to avoid running into the exact same issue we're facing here - libraries not respecting contractual boundaries.

In the case of your config, you could have either put the Sentry plugin last, or used libraries that respect the fact that Next.js configs may export functions. (in your config for example, @next/bundle-analyzer doesn't do that - you can either file an issue in their repository that you want that plugin to be able to handle that case out of the box, or you configure it like they suggest)

@gthb
Copy link
Contributor

gthb commented Dec 12, 2022

As a library maintainer, I am well aware of what breaking means. :)

Haha, sorry about that :-) — it seemed like we were applying different meanings, because the change was clearly breaking under the assumption that my existing config was valid. But it was that assumption of mine which didn't hold. Thanks for that, and for the explanatory comments on Next.js config composition, that's exactly the understanding I was missing.

@gthb
Copy link
Contributor

gthb commented Dec 13, 2022

Follow-up question: the withSentryConfig.d.ts declaration file still declares the return type of withSentryConfig to be NextConfigFunction | NextConfigObject, but the implementation now unconditionally returns a function. Was this just an oversight, or was the return type deliberately left broader to leave the option open to maybe return an object again in a later version?

@lforst
Copy link
Member

lforst commented Dec 13, 2022

Follow-up question: the withSentryConfig.d.ts declaration file still declares the return type of withSentryConfig to be NextConfigFunction | NextConfigObject, but the implementation now unconditionally returns a function. Was this just an oversight, or was the return type deliberately left broader to leave the option open to maybe return an object again in a later version?

Your assumption is correct. I deliberately left this return type broad so that we can go back to returning an object in future versions without requiring a major bump if we deem it necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants