vordgi / nimpl-classnames-minifier

Library for configuring style (css/scss/sass) modules to generate compressed classes in next.js
https://www.npmjs.com/package/@nimpl/classnames-minifier
MIT License
32 stars 1 forks source link

Cache warning and wrong styles #116

Closed KristenWilde closed 2 weeks ago

KristenWilde commented 2 weeks ago

We would love to use this package, but we're occasionally getting styles that are just plain wrong. Different production environments get different wrong styles, although they do seem to stick from one deploy to the next.

I noticed this warning logged while building the app for production. Is this expected?

#19 0.462 $ next build
#19 1.277 classnames-minifier: Can not find the package cache manifest at /app/.next/cache/ncm/ncm-meta/manifest.json
#19 1.277 Cleaning the dist folder...
#19 1.281 classnames-minifier: Dist folder cleared

We're on Next version 13.5.0

vordgi commented 2 weeks ago

What do you mean by "just plain wrong"?

KristenWilde commented 2 weeks ago

I haven't investigated as thoroughly as possible, but it seems that classes are being applied that shouldn't be.

vordgi commented 2 weeks ago

Is this expected?

Partially. If this is your first time building app with the package enabled, it will need to clear all next.js cache. To ensure that all files will be created with the correct configuration. But it seems like something going wrong in your case and it's being cleared all the time

vordgi commented 2 weeks ago

How does the built site looks like? Is everything visually right?

vordgi commented 2 weeks ago

You can also pass disableDistDeletion: true and control cache deletion on your own

KristenWilde commented 2 weeks ago

Most things are right visually, but there are some things that are off. Here's an example for an element that shouldn't have a margin. It has class .af, and that class is used twice in different css chunks.

Screenshot 2024-10-25 at 10 44 27 AM

This class should be applied:

Screenshot 2024-10-25 at 10 49 38 AM

This class should not be applied:

Screenshot 2024-10-25 at 10 48 57 AM

Thank you so much for the quick replies, by the way! ❤️

vordgi commented 2 weeks ago

Would it be possible to create a simple repro for this?

vordgi commented 2 weeks ago

You are using the latest version of the package (3.2.1) with a clean configuration. Right?

KristenWilde commented 2 weeks ago

Yes, we're using 3.2.1. Our next.config looks like this:

const nextConfig = {
  async headers() {
    return [
      {
        source: '/:path*',
        headers: [
          {
            key: 'Surrogate-Key',
            value: 'app:web-next',
          },
          {
            key: 'Vary',
            value: 'Device-Type',
          },
          {
            key: 'Surrogate-Control',
            value: 'max-age=31556952, stale-while-revalidate=604800, stale-if-error=1209600', // cache for 1 year, serve stale for 1 week for revalidation, stale for 2 weeks if error
          }
        ],
      },
      {
        source: '/api/auth/:slug',
        headers: [
          {
            key: 'Cache-Control',
            value: 'no-store, max-age=0',
          },
          {
            key: 'Surrogate-Control',
            value: 'no-store, max-age=0',
          },
        ],
      },
    ];
  },

  webpack: (config, options) => {
    config.plugins.push(
      new options.webpack.DefinePlugin({
        'process.env.NEXT_IS_SERVER': JSON.stringify(
          options.isServer.toString()
        ),
      })
    );

    return config;
  },

  async redirects() {
    return [
      {
        source: '/claim/add',
        destination: '/claim',
        permanent: false,
      },
    ];
  },

  images: {
    domains: [
      'photos.thedyrt.com',
      'photos-staging.thedyrt.com',
      'assets.thedyrt.com',
      'api.mapbox.com',
      'localhost',
    ],
    deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
  },

  sassOptions: {
    prependData: `$public: "${process.env.NEXT_PUBLIC_ASSETS || ''}";`,
  },

  eslint: {
    dirs: ['components', 'pages', 'utils', 'contexts', 'hooks', 'constants'],
  },
  staticPageGenerationTimeout: 360,
};

if (UI_ENV !== 'development') {
  nextConfig.assetPrefix = 'https://assets.thedyrt.com/next';
}

const sentryWebpackPluginOptions = {
  rewrite: true,
  ignore: ['node_modules'],
  setCommits: 'auto',
  release: sentryRelease,
};

const {
  PHASE_PRODUCTION_SERVER,
  PHASE_DEVELOPMENT_SERVER,
} = require('next/constants');

module.exports = (phase) =>
  classnamesMinifier({
    disabled:
      phase === PHASE_DEVELOPMENT_SERVER || phase === PHASE_PRODUCTION_SERVER,
  })(withSentryConfig(withImages(nextConfig), sentryWebpackPluginOptions));
vordgi commented 2 weeks ago

Okay, got it. Thanks for the issue, I will take a look on this!

vordgi commented 2 weeks ago

Can you try this configuration?

classnamesMinifier({
  disabled:
    phase === PHASE_DEVELOPMENT_SERVER || phase === PHASE_PRODUCTION_SERVER,
  experimental: {
    freedNamesPolicy: 'block',
  },
})
vordgi commented 2 weeks ago

And one more question. How exactly did this happen?

Is this repeated situation? Does this only happen in dev/prod? Is this the first time in a hundred builds? Is this the first build ever? Or do you build, then changed something, then rebuild?

vordgi commented 2 weeks ago

Version 4.0.0 released! Changed many default behaviors. First of all, auto-sync (which was the reason of this behavior) is disabled by default and marked as experimental. Also made package more stable and understandable. Read more about new configuration on the documentation page.

Thanks again for the issue, I hope everything will be fine now. Be free to create any issues or share ideas!

KristenWilde commented 2 weeks ago

@vordgi thank you!!! I look forward to testing it out later this week!