symfony / webpack-encore

A simple but powerful API for processing & compiling assets built around Webpack
https://symfony.com/doc/current/frontend.html
MIT License
2.23k stars 198 forks source link

Images in >0.30 in manifest have hashed path #808

Open darrylhein opened 4 years ago

darrylhein commented 4 years ago

When I update to >=0.30 (works fine in 0.29.1) the keys for images in the manifest are the hashed versions instead of the original paths. (I suspect fonts will be the same.) Prior to 0.30 I get:

"images/icons.svg": "/build/images/icons.677433a0.svg",

After 0.30 I get:

"images/icons.677433a0.svg": "/build/images/icons.677433a0.svg",

...which of course doesn't work :(

I'm import the image in my JS files.

I'm wondering if this is related to #771 though I'm still have the issue with 0.30.2.

I can reproduce the issue when updating webpack-encore (and sass-loader) on https://github.com/xmmedia/starter_wordpress

Let me know what other information you need.

stof commented 4 years ago

@darrylhein can you share the diff of your yarn.lock file for this upgrade (https://www.npmjs.com/package/yarn-lock-diff can probably be used to generate a summary of the upgrade) ? This issue might be related to a upgrade of one of a dependency rather than a change in Encore itself.

Lyrkan commented 4 years ago

As discussed on Slack with @4c0n, it seems to be related to https://github.com/danethurber/webpack-manifest-plugin/issues/209

We updated the file-loader in v0.29.0 which came with its esModule option set to true by default (so it should be broken in v0.29 as well).

A quick workaround would be to change that value to false using configureUrlLoader() (since we don't have a configureFileLoader()):

Encore.configureUrlLoader({
  images: {
    limit: 0, // Avoids files from being inlined
    esModule: false
  }
});
4c0n commented 4 years ago

@Lyrkan Thanks for looking into it so fast :smile: I'm just using copyFiles() in the webpack.config.js file to get the right result for now.

darrylhein commented 4 years ago

@stof Here's the "diff". I only changed the encore version in the package.json and then did yarn install in the hope to not update anything else unrelated.

yarn.lock diff

I've tried @Lyrkan's suggestion, but didn't fix things and it appears to have removed the image files from the manifest all together. I could just copy the files, but I'd like them to be run through the svgo-loader.

Thanks for the help so far.

Lyrkan commented 4 years ago

I've tried @Lyrkan's suggestion, but didn't fix things and it appears to have removed the image files from the manifest all together

With the limit option set to 0 (or falseor any value lower than your filesize)? If you don't set it, your files will be inlined as a base64 string, so no output file will be generated (and nothing will appear in the manifest).

darrylhein commented 4 years ago

Ah! I half ignored your code. Putting in just limit: 0 seems to resolve the issue – file is in the manifest without a hash and contains the right content. In my case, esModule: false doesn't seem to be required. (In debugging, I also disabled the svgo-loader but didn't seem to make a diff.)

I'm not exactly sure what this means – do we call this resolved? Should we document this somewhere? Or is it just something specific to my project config?

Lyrkan commented 4 years ago

Putting in just limit: 0 seems to resolve the issue

That'd depend on the version of the url-loader you added to your dependencies I guess (esModule defaults to true since v3.0.0).

I'm not exactly sure what this means – do we call this resolved?

Nah, that's definitely a bug, not sure what we can do on our side though (apart from setting esModule to false by default for the file-loader... which would, in my opinion, be a step backward).

4c0n commented 4 years ago

"By default, file-loader generates JS modules that use the ES modules syntax. There are some cases in which using ES modules is beneficial, like in the case of module concatenation and tree shaking."

https://webpack.js.org/loaders/file-loader/

I'm not really understanding why exactly it's causing the issue at hand though. The description seems rather unrelated?

4c0n commented 4 years ago

If it's actually file-loader that's causing the issue, then blacklisting the versions that cause the problem and reporting/fixing the problem on that end seems the best course of action IMO.

Lyrkan commented 4 years ago

If it's actually file-loader that's causing the issue

I don't think the file-loader has a problem, only that the webpack-manifest-plugin is not compatible with the esModule mode of it yet (enabled by default since file-loader@5.0.0). There is already an issue opened about that (see my first post).

4c0n commented 4 years ago

Alright, how about excluding the version until it's fixed then? Currently there apparently are compatibility issues that prevent this project from working properly. We should not just ignore that until the problem goes away, right?

Lyrkan commented 4 years ago

Alright, how about excluding the version until it's fixed then?

Sadly this isn't that simple.

There is no reason to exclude a version of the file-loader since the latest one should be compatible with the manifest plugin. What's not compatible is the esModule option when it's set to true, the bug isn't present if it is set to false.

We could change that default value to false quite easily but it would mean that everyone will lose the benefits of the esModule mode by default, even if they are not impacted by that bug (I don't know how many people import images from their JS code and then try to read their path from the manifest).

Also it could introduce breaking changes since ES6 imports do not always behave the same way than CommonJS imports (see my comment here). Some people already had to change their code when that feature was introduced in 0.29.0.

At the end of the day, yes, that's definitely an issue, but there are some workarounds while waiting for it to be fixed upstream:

4c0n commented 4 years ago

I mean excluding the version of webpack-manifest-plugin that introduced the issues and wait for them to fix it. How is that not an option? Sure workarounds exist, but that doesn't change the fact that currently it isn't working correctly out-of-the-box and it seems it could be. Saving us all the trouble of rediscovering the same problem and having to go look into it /for a workaround.

Lyrkan commented 4 years ago

I mean excluding the version of webpack-manifest-plugin that introduced the issues and wait for them to fix it. How is that not an option?

Because all versions of the webpack-manifest-plugin probably have this issue.

stof commented 4 years ago

it is not webpack-manifest-plugin introducing the issue. It is webpack-manifest-plugin not being updated to support the esModule mode of the file-loader.

4c0n commented 4 years ago

Alright I see the issue now.

Had a quick look at the plugin repo and there hasn't been any activity there since December 2019. The author/maintainer isn't responding to the issue at all.

Looks like the package will be abonded if it's not already. 😭

ju1ius commented 4 years ago

@4c0n & @Lyrkan : You might want to look into webdeveric/webpack-assets-manifest. It is actively maintained and looks like it has all the features you need.

As an example, the following config:

// webpack.config.js
const MiniCssExtractPlugin = require('mini-css-extract-plugin')
const WebpackAssetsManifest = require('webpack-assets-manifest')

module.exports = {
  entry: {
    front: './front.js',
  },
  output: {
    filename: 'js/[name].[contenthash].js',
    publicPath: '/assets/dist/',
  },
  optimization: {
    runtimeChunk: 'single',
  },
  plugins: [
    new MiniCssExtractPlugin({
      filename: 'css/[name].[contenthash].css',
    }),
    new WebpackAssetsManifest({
      publicPath: true,
      entrypoints: true,
      integrity: true,
    }),
  ],
  module: {
    rules: [
      {
        test: /\.css$/,
        use: [
          {loader: MiniCssExtractPlugin.loader},
          {loader: 'css-loader', options: {url: true}},
        ]
      },
      {
        test: /\.(jpe?g|png|svg|gif)$/,
        use: [
          {loader: 'file-loader', options: {name: 'img/[name].[contenthash].[ext]'}},
        ],
      },
    ],
  },
}
// front.js
import './front.css'
/* front.css */
.foo {
  background: url("example.jpg");
}

Produces the following manifest:

{
  "entrypoints": {
    "front": {
      "js": [
        "/assets/dist/js/runtime.67a91499be4cb5c00239.js",
        "/assets/dist/js/front.8699e2b0dd20e58cd32a.js"
      ],
      "css": [
        "/assets/dist/css/front.dfe5f8bceb27844d87f9.css"
      ]
    }
  },
  "front.css": {
    "src": "/assets/dist/css/front.dfe5f8bceb27844d87f9.css",
    "integrity": "sha256-hM/uAmUiFoVk54BH7wSa67FQOzYVMfL1dmVF8AOB1Xc= sha384-S/Pezexz+pGQ4igWksSiNbnJP6dueIPuw9JQdVvfIcRU6lOlCV7wjxov0Wyda6SJ sha512-79NZNneIGLgIJCrwA9TIZbcW0lKgypFzvI+nH+nd3NUV2KOdqH9A+XDuh2UBW+dpAQcqBPr2jyJDry505Jg4FQ=="
  },
  "front.js": {
    "src": "/assets/dist/js/front.8699e2b0dd20e58cd32a.js",
    "integrity": "sha256-GK5U8n5yDd5kUpE0bq8Mi2LiZTBD+i8EtLBKfL6uIIg= sha384-Oa1vMIiynZKkI7VZd4/LaVbiOmyrfWWNgEILZXyUJK/oGEsANK7GoQQOSfO5TsLP sha512-ZFQCD/gud1Sf8vEPGeDEHNH8KBqDGjCplqALEhVY4WESHDGB0zbCK1IZbx6hxi/JfLzCdjYNk1HH/YLqmxa4KA=="
  },
  "img/example.jpg": {
    "src": "/assets/dist/img/example.31d6cfe0d16ae931b73c59d7e0c089c0.jpg",
    "integrity": "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU= sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb sha512-z4PhNX7vuL3xVChQ1m2AB9Yg5AULVxXcg/SpIdNs6c5H0NE8XYXysP+DGNKHfuwvY7kxvUdBeoGlODJ6+SfaPg=="
  },
  "runtime.js": {
    "src": "/assets/dist/js/runtime.67a91499be4cb5c00239.js",
    "integrity": "sha256-B5W3PUqfVixDW5RqL7xk9wbdMA7BtOyW9XO6++la61I= sha384-bQ4/nZ2qJ9hFDPtL1EAWTWhBJMJjraH6ja53ndlszok/VzFHK8NGTChLVGkdVG1X sha512-EPbDbwy4jKbZVDOpi0aHuzOtqT1twmX9tRWObHO1N0/K9UC4TXp5adZoh9AilXBuh/ayW4BOajsgO4JGk8f1fw=="
  }
}
4c0n commented 4 years ago

Seems great to me. The variant that is currently used is abandoned, I tried reaching out to both the owner, who said he would try to get in touch with the maintainers, and the last maintainer, but that didn't lead to anything productive unfortunately.

misaon commented 3 years ago

Some update guys?

ureimers commented 3 years ago

I think we're having the same problem. Until now we used:

const SVGSpriteMapPlugin = require("svg-spritemap-webpack-plugin");

Encode.addPlugin(new SVGSpriteMapPlugin('./public/bundles/mybundle/icons/**/*.svg', {
        output: {
            filename: Encore.isProduction()
                ? 'icons/spritemap.[contenthash].svg'
                : 'icons/spritemap.svg',
        },
        ...
    }))

And now manifest.json has:

mybundle/build/icons/spritemap.e679d260b3aabc45.svg": "/mybundle/build/icons/spritemap.e679d260b3aabc45.svg

At the end of the day, yes, that's definitely an issue, but there are some workarounds while waiting for it to be fixed upstream:

  • setting the esModule option of the file-loader to false: there isn't any shortcut for it in Encore atm, but there are ways to do it (for instance by using Encore.configureLoaderRule(...) or by changing the generated config)
  • using the url-loader instead of the file-loader (using Encore.configureUrlLoader(...)) with the right values for the limit and esModule options (depending on the version you added to your package.json)

@Lyrkan Could you please give me an example on how to disable the esModule option? configureUrlLoader was removed with v1.0 and I don't know what to pass to Encore.configureLoaderRule.

ureimers commented 3 years ago

I found another fix/workaround:

It's based on the new removeKeyHash option of the webpack-manifest-plugin. That option was introduced in an update (https://github.com/shellscape/webpack-manifest-plugin/commit/5bad3ea98861ef750954e8c58ee6ea21d3592845) to fix https://github.com/shellscape/webpack-manifest-plugin/issues/210 and with that also https://github.com/shellscape/webpack-manifest-plugin/issues/209 (which @Lyrkan referenced above, but at a time where the fix didn't yet exist):

    Encore.configureManifestPlugin(function(options) {
        options.removeKeyHash = /(?<=\.)([a-f0-9]{8}){1,4}\./;
    })

The default reges of the removeKeyHas option is /([a-f0-9]{32}\.?)/gi which only removes 32 character long hashes (e.g., md5). We're having 16 character long hashes in our projects.

The regex I use will work for 8, 16, 24 and 32 character long hashes so you, @darrylhein, should be able to use it too, as your hashes seem to be 8 characters long.

This still feels a little weird but it works and I don't have to disable the esModule module option (which I wasn't able to accomplish anyway).