Skip to content

[Enhancement] Switch WebpackManifestPlugin for remote package and apply remove hash hack for CopyWebpackPlugin files #1092

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[Enhancement] Switch WebpackManifestPlugin for remote package and apply remove hash hack for CopyWebpackPlugin files #1092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1111242
use WebpackManifestPlugin and apply the file versioning hack
shadowc Feb 14, 2022
91878c6
fix tests
shadowc Feb 14, 2022
63999e8
using a hash of 8 chars
shadowc Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/plugins/manifest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
'use strict';

const WebpackConfig = require('../WebpackConfig'); //eslint-disable-line no-unused-vars
const { WebpackManifestPlugin } = require('../webpack-manifest-plugin');
const { WebpackManifestPlugin } = require('webpack-manifest-plugin');
const PluginPriorities = require('./plugin-priorities');
const applyOptionsCallback = require('../utils/apply-options-callback');
const copyEntryTmpName = require('../utils/copyEntryTmpName');
Expand All @@ -33,7 +33,15 @@ module.exports = function(plugins, webpackConfig) {
const isJsOrJsMapFile = /\.js(\.map)?$/.test(file.name);

return !isCopyEntry && !(isStyleEntry && isJsOrJsMapFile);
}
},
map: (file) => {
if (webpackConfig.useVersioning) {
// Remove hash in manifest key
file.name = file.name.replace(/(\.[a-f0-9]{8})(\..*)$/, '$2');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @shadowc,

I'm not sure it would always work... IIRC the hard thing to handle here is that the user has control on the naming, so you can't really assume that:

  • hashes will be 8 characters long (users could chose a different length)
  • hashes will only be composed of hexadecimal characters (hash function can be changed as well)
  • hashes will be at that exact position (eg. [contenthash].[name].js)
  • original filenames won't contain something that looks like a hash (eg. foo.aaaaaaaa.d1fc2a.txt would become foo.d1fc2a.txt)

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the heads up! We do have the info at that point of what format the user choose for the to file name, do we? Also, there is a default format [name].[contenthash].[ext] that encore uses?

I'll investigate this further and put against several scenarios to double check!

Copy link
Author

Choose a reason for hiding this comment

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

To answer my own question, I believe we don't have all this information.

I will think of ways to cache regexp patterns for each CopyWebpackPlugin entry together with the position of the hash, then use this to remove such hash from the entry in this function. I believe this could work.

Copy link
Author

Choose a reason for hiding this comment

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

On the other hand, what I need to cache is the name of the original entry file (not the to field) and just retrieve that somehow.

Copy link
Author

Choose a reason for hiding this comment

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

Here's a simplified version of how this should work:

static_file_mapping = {};
config = {
  plugins: [
    new CopyPlugin([{
            from: {
                glob: 'static/**/*.@(png|svg|jpg|gif|ico)',
                dot: false,
                noext: false,
            },
            to: '[contenthash].[ext]',
            // flatten: true
            transformPath(targetPath, absosutePath) {
                relative_path = path.relative(
                    path.resolve(root_dir),
                    absosutePath)
                static_file_mapping[relative_path] = targetPath;
                return targetPath;
            }
    }]),
    new ManifestPlugin({
      assets: static_file_mapping,
    })
  ]
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

If that helps here is the PR in which I initially tried to implement copyFiles() using the WebpackManifestPlugin: #221

As you will be able to read in the comments I couldn't make it work at that time because the CopyWebpackPlugin didn't expose original filenames. It looks like some work as been done since then: webpack-contrib/copy-webpack-plugin#309 (comment)

Not sure if that can be used without modifying the WebpackManifestPlugin internals though.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I'll look into this and come up with a better solution!

}

return file;
},
};

manifestPluginOptions = applyOptionsCallback(
Expand Down
21 changes: 0 additions & 21 deletions lib/webpack-manifest-plugin/LICENSE
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions lib/webpack-manifest-plugin/README.md
View file
Open in desktop

This file was deleted.

115 changes: 0 additions & 115 deletions lib/webpack-manifest-plugin/helpers.js
View file
Open in desktop

This file was deleted.

145 changes: 0 additions & 145 deletions lib/webpack-manifest-plugin/hooks.js
View file
Open in desktop

This file was deleted.

73 changes: 0 additions & 73 deletions lib/webpack-manifest-plugin/index.js
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"webpack": "^5.35",
"webpack-cli": "^4.9.1",
"webpack-dev-server": "^4.0.0",
"webpack-manifest-plugin": "^4.1.1",
"yargs-parser": "^20.2.4"
},
"devDependencies": {
Expand Down
Loading