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

Incorrect filename temlate generation behavior #334

Closed
RAX7 opened this issue Aug 27, 2022 · 10 comments · Fixed by #339
Closed

Incorrect filename temlate generation behavior #334

RAX7 opened this issue Aug 27, 2022 · 10 comments · Fixed by #339

Comments

@RAX7
Copy link
Contributor

RAX7 commented Aug 27, 2022

Bug report

When using .svg files with squooshMinify or sharpMinify (which do not support the svg format), an invalid filename is generated from the filename option.

webpack config:

export default {
  mode: 'production',

  entry: {
    main: path.resolve(__dirname, '../src/scripts/index.js'),
  },

  output: {
    path: path.join(__dirname, '../build'),
    filename: 'js/[name].js',
    clean: true,
  },

  optimization: {
    minimizer: [
      new ImageMinimizerPlugin({
        minimizer: {
          filename: 'minified-xxx-[name]-yyy[ext]',
          implementation: ImageMinimizerPlugin.squooshMinify,
          options: {},
        },
      }),
    ],
  }
};

Actual Behavior

create file:
build/minified-xxx-minified-xxx-minified-xxx-minified-xxx-minified-xxx-minified-xxx-minified-xxx-minified-xxx-svg-file-yyy-c8e115cf-yyy-yyy-yyy-yyy-yyy-yyy-yyy.svg

Expected Behavior

create file:
build/assets/minified-xxx-svg-file-yyy-c8e115cf.svg

How Do We Reproduce?

import svg from './svg-file.svg';

Please paste the results of npx webpack-cli info here, and mention other relevant information

  System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 Intel(R) @ 3.30GHz
    Memory: 1.73 GB / 7.97 GB
  Binaries:
    Node: 16.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.15.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (104.0.1293.70)
    Internet Explorer: 11.0.19041.1566
  Packages:
    babel-loader: ^8.2.5 => 8.2.5
    copy-webpack-plugin: ^11.0.0 => 11.0.0
    css-loader: ^6.7.1 => 6.7.1
    eslint-webpack-plugin: ^3.2.0 => 3.2.0
    html-loader: ^4.1.0 => 4.1.0
    html-webpack-plugin: ^5.5.0 => 5.5.0
    image-minimizer-webpack-plugin: ^3.3.0 => 3.3.0
    postcss-loader: ^7.0.1 => 7.0.1
    sass-loader: ^13.0.2 => 13.0.2
    style-loader: ^3.3.1 => 3.3.1
    stylelint-webpack-plugin: ^3.3.0 => 3.3.0
    webpack: ^5.73.0 => 5.73.0
    webpack-cli: ^4.10.0 => 4.10.0
    webpack-dev-server: ^4.9.3 => 4.9.3
    webpack-merge: ^5.8.0 => 5.8.0
@alexander-akait
Copy link
Member

alexander-akait commented Aug 29, 2022

Sounds like you apply the loader multiple times... Can you provide a full configuration, because I can't reproduce, thank you

@RAX7
Copy link
Contributor Author

RAX7 commented Aug 29, 2022

webpack/webpack.common.js

import path from 'path';
import url from 'url';

import CopyWebpackPlugin from 'copy-webpack-plugin';
import HtmlWebpackPlugin from 'html-webpack-plugin';

const __filename = url.fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

export default {
  entry: {
    main: path.resolve(__dirname, '../src/scripts/index.js'),
  },
  output: {
    path: path.join(__dirname, '../build'),
    filename: 'js/[name].js',
    clean: true,
  },
  optimization: {
    splitChunks: {
      chunks: 'all',
      cacheGroups: {
        commons: {
          name: 'commons',
          chunks: 'initial',
        },
      },
    },
  },
  plugins: [
    new CopyWebpackPlugin({
      patterns: [{ from: path.resolve(__dirname, '../public'), to: 'public' }],
    }),
    new HtmlWebpackPlugin({
      template: path.resolve(__dirname, '../src/index.html'),
    }),
  ],
  resolve: {
    alias: {
      '~': path.resolve(__dirname, '../src'),
    },
  },
  module: {
    rules: [
      {
        test: /\.mjs$/,
        include: /node_modules/,
        type: 'javascript/auto',
      },
      {
        test: /\.html$/i,
        loader: 'html-loader',
      },
      {
        test: /\.(ico|jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2)(\?.*)?$/,
        type: 'asset/resource',
      },
    ],
  },
};

webpack/webpack.config.prod.js

import webpack from 'webpack';
import { merge } from 'webpack-merge';
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
import ImageMinimizerPlugin from 'image-minimizer-webpack-plugin';
import common from './webpack.common.js';

const optimization = {
  minimizer: [
    new ImageMinimizerPlugin({
      minimizer: {
        filename: 'minified-xxx-[name]-yyy[ext]',
        // filename: 'minified-[name]-[width]x[height][ext]',
        implementation: ImageMinimizerPlugin.squooshMinify,
        options: {
          encodeOptions: {
            jpeg: {
              quality: 90,
            },
            webp: {
              quality: 90,
            },
            avif: {
              quality: 90,
            },
          },
        },
      },
      generator: [
        {
          preset: 'webp',
          implementation: ImageMinimizerPlugin.squooshGenerate,
          filename: 'generated-[name]-[width]x[height][ext]',
          options: {
            encodeOptions: {
              webp: {
                quality: 90,
              },
            },
          },
        },
        {
          preset: 'avif',
          implementation: ImageMinimizerPlugin.squooshGenerate,
          filename: 'generated-[name]-[width]x[height][ext]',
          options: {
            encodeOptions: {
              avif: {
                quality: 90,
              },
            },
          },
        },
      ],
    }),
  ],
};

export default merge(common, {
  mode: 'production',
  // devtool: 'source-map',
  stats: 'errors-only',
  bail: true,
  optimization,
  output: {
    filename: 'js/[name].[chunkhash:8].js',
    chunkFilename: 'js/[name].[chunkhash:8].chunk[ext]',
    assetModuleFilename: 'assets/[name]-[contenthash:8][ext]',
  },
  plugins: [
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify('production'),
    }),
    new MiniCssExtractPlugin({
      filename: 'css/[name].[chunkhash:8].css',
      chunkFilename: 'css/[name].[chunkhash:8].chunk[ext]',
    }),
  ],
  module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        use: 'babel-loader',
      },
      {
        test: /\.s?css/i,
        use: [MiniCssExtractPlugin.loader, 'css-loader', 'postcss-loader', 'sass-loader'],
      },
    ],
  },
});

package.json

{
  "name": "webpack-starter",
  "version": "1.0.0",
  "type": "module",
  "description": "A light foundation for your next frontend project based on webpack.",
  "scripts": {
    "lint": "npm run lint:styles; npm run lint:scripts",
    "lint:styles": "stylelint src",
    "lint:scripts": "eslint src",
    "build": "cross-env NODE_ENV=production webpack --config webpack/webpack.config.prod.js",
    "start": "webpack serve --config webpack/webpack.config.dev.js"
  },

  "license": "MIT",
  "devDependencies": {
    "@babel/core": "^7.18.6",
    "@babel/eslint-parser": "^7.18.2",
    "@babel/plugin-proposal-class-properties": "^7.18.6",
    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
    "@babel/preset-env": "^7.18.6",
    "@squoosh/lib": "^0.4.0",
    "autoprefixer": "^10.4.7",
    "babel-loader": "^8.2.5",
    "copy-webpack-plugin": "^11.0.0",
    "cross-env": "^7.0.3",
    "css-loader": "^6.7.1",
    "eslint": "^8.20.0",
    "eslint-webpack-plugin": "^3.2.0",
    "html-loader": "^4.1.0",
    "html-webpack-plugin": "^5.5.0",
    "image-minimizer-webpack-plugin": "^3.3.0",
    "imagemin": "^8.0.1",
    "imagemin-avif": "^0.1.3",
    "imagemin-webp": "^7.0.0",
    "mini-css-extract-plugin": "^2.6.1",
    "node-sass": "^7.0.1",
    "postcss": "^8.4.14",
    "postcss-loader": "^7.0.1",
    "postcss-preset-env": "^7.7.2",
    "sass-loader": "^13.0.2",
    "style-loader": "^3.3.1",
    "stylelint": "^14.9.1",
    "stylelint-config-standard-scss": "^5.0.0",
    "stylelint-webpack-plugin": "^3.3.0",
    "webpack": "^5.73.0",
    "webpack-cli": "^4.10.0",
    "webpack-dev-server": "^4.9.3",
    "webpack-merge": "^5.8.0"
  },
  "dependencies": {
    "core-js": "^3.23.5"
  }
}

@alexander-akait
Copy link
Member

Try to remove filename: 'minified-xxx-[name]-yyy[ext]', from minified, I don't think you need rename file after minication, assets modules already rename file as you want, this option for exotic cases

@alexander-akait
Copy link
Member

If it will not help, can you create reproducible github repo, hard to use code from comments, thank you

@RAX7
Copy link
Contributor Author

RAX7 commented Aug 30, 2022

Here is repo with bug

Try to remove filename

No, this does not affect. I have added an option exclude: /\.svg$/i to avoid this behavor, but I still think this a bug.

@alexander-akait
Copy link
Member

alexander-akait commented Sep 5, 2022

I see, if you don't do something with files/assets you should exclude them from a plugin, i.e. use /\.(jpe?g|png|gif|tif|webp|avif|jxl)$/i (no svg there), otherwise you will spend build time on nothing (we can't know all possible extensions and different tools support different extensions). Or use exclude as you written above.

But even if we skip them you still will have renamed file, because you set filename (I think you are not expecting this).

@alexander-akait
Copy link
Member

alexander-akait commented Sep 5, 2022

@RAX7 So I don't have a strong opinion that we have to do something here, although we can update docs and exclude the svg extension for sharp and squoosh

But I see the DX problem here

@alexander-akait
Copy link
Member

Please try https://github.com/webpack-contrib/image-minimizer-webpack-plugin/releases/tag/v3.3.1, anyway I strong recommend to exclude svg for better perf

@RAX7
Copy link
Contributor Author

RAX7 commented Sep 6, 2022

@alexander-akait I've tried v3.3.1 and it's works fine for me. Thank you!

So I don't have a strong opinion that we have to do something here

In my opinion, we have only two options: skip the file or throw an error. But in this way, I think there will be a problem with multiple minimizers option. For example: it should be possible to mix the sharp or squoosh minimizer for bitmaps formats and imagemin-svgo for svg. And if first minimizer mark file as unsupported (original.info.original = true) then second one will also skip this file.

Maybe better fix will be move normalizeProcessedResult function outside loop to normalize whole batch result. But I not sure.

@alexander-akait
Copy link
Member

@RAX7 Yeah, feel free to send a PR if you have more ideas, currently I jsut skip any actions when file was no changed

RAX7 added a commit to RAX7/image-minimizer-webpack-plugin that referenced this issue Sep 8, 2022
alexander-akait pushed a commit that referenced this issue Sep 9, 2022
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 a pull request may close this issue.

2 participants